Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant new feature: offline leveling. Players can now use a chat command to leave their character actively grinding in the game world even after disconnecting, leveraging their MU Helper settings. This is achieved through a 'ghost' player system that handles combat, item pickup, and movement, while minimizing network overhead. The feature also includes automatic session termination upon player re-login and refactors MU Helper configurations for clarity. Additionally, a quality-of-life improvement for administrators has been added, allowing for easy duplication of items within the admin panel. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a significant new feature: offline leveling. The implementation is comprehensive, covering the AI for the offline character, state management, and integration with existing systems. The use of DispatchProxy to suppress network views is a clever approach. Additionally, the PR includes some refactoring and a new item duplication feature in the admin UI. My review identified a few areas for improvement: a minor file header inconsistency, a maintainability issue with a hardcoded string for skill identification, and a potential bug in the new item duplication logic that could lead to data issues. I've provided suggestions for each of these points.
There was a problem hiding this comment.
Code Review
This pull request introduces a significant new feature: offline leveling. The implementation is comprehensive, including a ghost player entity, a server-side AI to control it, and plugins to manage its lifecycle. The approach to suppress network traffic for the ghost player using a null-view proxy is clever. Additionally, the PR includes UI enhancements like a 'Duplicate' button for items in the admin interface. While the core feature implementation is solid, I've identified a critical bug in the new 'Duplicate' item functionality that could lead to data corruption, along with a couple of medium-severity maintainability concerns in the new code that would be beneficial to address.
There was a problem hiding this comment.
Code Review
This pull request introduces the significant 'Offline Leveling' feature, allowing players to continue grinding after disconnecting. The implementation is comprehensive, including the AI for the offline player, management of sessions, and seamless integration with the login process. The use of a DispatchProxy to suppress network traffic for offline players is a clever approach. Additionally, the PR adds a 'Duplicate' item feature in the admin panel's inventory editor and a 'Refresh' button on edit pages. Several smaller refactorings improve code style and robustness. My review includes a couple of suggestions to enhance maintainability by refactoring large classes and better separating business logic from UI components. Overall, this is a solid contribution with well-executed features.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a significant new feature: offline leveling. The implementation is well-structured, creating a 'ghost' player driven by a server-side AI that mimics the MU Helper functionality. The use of a null-view proxy to suppress network traffic is a clever approach. The supporting changes, such as the admin panel refresh and duplicate item buttons, are also well-implemented. I've found one potential issue regarding the game's economy that should be considered.
…gic into specific handlers
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-implemented offline leveling feature, allowing players to continue progressing while disconnected. The implementation is robust, using a 'ghost' player with suppressed network views and a modular AI system for combat, buffing, and other actions. The addition of 'Refresh' and 'Duplicate' buttons to the admin panel are also valuable usability improvements. The code is generally of high quality with good structure and modern C# practices. I have one minor suggestion regarding a hardcoded future date in an update plugin.
| public override bool IsMandatory => true; | ||
|
|
||
| /// <inheritdoc /> | ||
| public override DateTime CreatedAt => new(2025, 3, 11, 12, 0, 0, DateTimeKind.Utc); |
There was a problem hiding this comment.
There was a problem hiding this comment.
Haha, well we're in 2026, though 😉 Please adjust it if this update plugin is really needed.
There was a problem hiding this comment.
ops, I didn't notice the year 😅
There was a problem hiding this comment.
Code Review
This pull request introduces the significant 'Offline Leveling' feature, allowing players to continue progressing while disconnected. The implementation is well-structured, separating concerns into various handlers for combat, buffs, movement, and item management. The use of a 'ghost' player with a NullViewProxy to suppress network traffic is a clever and effective approach.
In addition to the main feature, the PR includes several valuable improvements:
- A 'Refresh' button in the admin panel to reload data.
- A 'Duplicate' button in the inventory editor for easier item management.
- Refinements to the Dockerfile for better layer caching and configuration.
- A bug fix in
ValueListWrapperfor safer item removal.
I have one suggestion for improving code clarity and safety in StatAttribute.cs by using a more modern C# pattern. Overall, this is a high-quality contribution with substantial new functionality and thoughtful improvements.
| /// Configuration for the <see cref="MuHelper"/>. | ||
| /// </summary> | ||
| public class MuHelperConfiguration | ||
| public class MuHelperServerConfiguration |
| public T? GetPlugIn<T>() | ||
| where T : class, IViewPlugIn | ||
| { | ||
| return System.Reflection.DispatchProxy.Create<T, NullViewProxy>(); |
There was a problem hiding this comment.
Would returning null work here? Then we wouldn't need the NullViewProxy.
There was a problem hiding this comment.
I tested with null and it works, I'll remove the Proxy.
| /// </summary> | ||
| /// <returns>The size of the inventory.</returns> | ||
| public byte GetInventorySize() | ||
| public byte InventorySize |
There was a problem hiding this comment.
Please move this property up to the other properties (SA1201)
| public override bool IsMandatory => true; | ||
|
|
||
| /// <inheritdoc /> | ||
| public override DateTime CreatedAt => new(2025, 3, 11, 12, 0, 0, DateTimeKind.Utc); |
There was a problem hiding this comment.
Haha, well we're in 2026, though 😉 Please adjust it if this update plugin is really needed.
There was a problem hiding this comment.
Is this really needed?
As far as I remember, missing plugin configurations are added automatically on server start at
MUnique.OpenMU.Startup.Program.PlugInConfigurationsFactory.
There was a problem hiding this comment.
I was thinking about that, but this way it's possible to persist the disabled plugin in case the admin doesn´t want to enable offlevel. Does that make sense?
| /// <summary> | ||
| /// The version of the <see cref="AddOfflineLevelingPlugIn"/>. | ||
| /// </summary> | ||
| AddOfflineLeveling = 68, |
There was a problem hiding this comment.
Remove, if plugin is not required.
|
|
||
| // StartAsync handles everything: login-server logoff, suppress event, | ||
| // disconnect real player, then spawn the ghost. | ||
| if (!await manager.StartAsync(player, loginName).ConfigureAwait(false)) |
There was a problem hiding this comment.
You could deserialize the mu helper settings here and pass them as arguments to the manager.
There was a problem hiding this comment.
I suggest some changes to this class:
- Rename to MuHelperSettings
- Move the serializer logic to a separate class into the GameServer project. The serialization should not be part of GameLogic.
Feature: Offline Leveling (/offlevel command)
Allows players to continue grinding after disconnecting. When a player types /offlevel, their character stays in the world as a ghost and automatically fights monsters using the existing MU Helper configuration.
How it works:
Technical details:
Feature: Add a "Refresh" button to admin interface to reload updated entities from the database.
Feature: Add a "Duplicate" button to inventory in admin interface.
PS. This also contains fixes from previous PR #706
PS2. This PR will not include party support, otherwise it would be too much to review.