Apply monster attribute changes instantly on the game server#785
Apply monster attribute changes instantly on the game server#785sven-n wants to merge 2 commits into
Conversation
Register spawned attackable NPCs for changes of their MonsterDefinition via the ConfigurationChangeMediator and reload the cached stat attributes of the MonsterAttributeHolder in place, so configuration changes to monster attributes take effect without restarting the server. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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 enables real-time updates for monster attributes on the game server. By introducing a reload mechanism that refreshes cached stat attributes in-place, administrators can now modify monster configurations via the admin panel and see those changes reflected immediately on existing spawned NPCs, preserving their current state such as health and position. Highlights
New Features🧠 You can now enable Memory (public preview) 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. 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 the 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 counterproductive. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to dynamically reload monster attributes from their configuration definitions without needing to respawn them. It adds an ApplyChanges method to MonsterAttributeHolder, registers configuration change listeners in MapInitializer, and includes unit tests to verify the behavior. The review feedback highlights a potential memory leak where configuration change registrations may not be disposed of if an NPC is disposed outside of mediator callbacks, suggesting a registration disposal mechanism on the NPC. Additionally, a performance optimization is recommended in MonsterAttributeHolder to prevent redundant dictionary rebuilds when multiple instances of the same monster apply changes simultaneously.
Let the NPC own the MonsterDefinition change registration so it is disposed when the NPC is disposed (e.g. dying non-respawning monster, map cleanup), preventing a memory leak. Also skip redundant stat dictionary rebuilds when multiple instances of the same monster apply the same change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Thanks for the review! Addressed all three findings in 030bd55:
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to dynamically reload monster attributes from configuration changes without requiring a server restart or monster respawn. It updates MonsterAttributeHolder to allow modifying its stat attributes, registers spawned monsters for configuration changes in MapInitializer, and ensures that these registrations are properly disposed of in AttackableNpcBase. Additionally, a comprehensive set of unit tests has been added in MonsterAttributeReloadTests.cs to verify this behavior. There are no review comments, so no further feedback is provided.
Summary
Closes #784.
Changing a monster's attributes (e.g. via the admin panel) previously had no effect until a server restart, because
MonsterAttributeHoldersnapshots the monster's stats from itsMonsterDefinitionat construction time into a static cache and never refreshed them.This change hooks into the existing
ConfigurationChangeMediator: whenever aMonsterAttributeis added/removed/modified, the infrastructure publishes a parent-level change for itsMonsterDefinition. Each spawned attackable NPC now registers for those changes and reloads its cached stat attributes in place — no re-spawn, so health and position are preserved.Changes
MonsterAttributeHolder: addedApplyChanges(), which rebuilds the stat dictionary from the (already updated in place)MonsterDefinition.Attributes, refreshes the shared static cache, and atomically swaps in the new dictionary.AttackableNpcBase: addedReloadAttributes()forwarding to the holder.MapInitializer.RegisterForConfigChanges: each spawned attackable NPC registers for changes to itsMonsterDefinition; the registration is disposed alongside the NPC in all existing teardown paths.Known limitation
Stats that currently have a per-instance composable element applied (e.g. an active magic effect, or the disabled-by-default
MonsterAttributeScaler) won't reflect a base-value change until that element is removed/re-added, since composable attributes snapshot their base at add time. This matches existing system behavior.Test plan
MonsterAttributeReloadTestscover changing an existing attribute value and adding a new attribute; both confirm the value takes effect afterReloadAttributes().🤖 Generated with Claude Code