-
-
Notifications
You must be signed in to change notification settings - Fork 85
refactor: remove ActiveMagicEffectsMap and related functionality #2398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Reviewed everything up to cfcc18e in 1 minute and 51 seconds
More details
- Looked at
485lines of code in8files - Skipped
0files when reviewing. - Skipped posting
13drafted comments based on config settings.
1. skymp5-server/cpp/server_guest_lib/ActiveMagicEffectsMap.cpp:1
- Draft comment:
Removed file ActiveMagicEffectsMap.cpp. Confirm that all calls to magic effects functions have been properly refactored and tests updated. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. skymp5-server/cpp/server_guest_lib/ActiveMagicEffectsMap.h:1
- Draft comment:
Removed ActiveMagicEffectsMap.h. Verify that no other modules rely on its interface. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. skymp5-server/cpp/server_guest_lib/AnimationSystem.cpp:25
- Draft comment:
Similarly, blockStop callback has commented out changes. Remove if no longer relevant. - Reason this comment was not posted:
Marked as duplicate.
4. skymp5-server/cpp/server_guest_lib/EatItemEvent.cpp:46
- Draft comment:
The call to ApplyMagicEffects is commented out; if the magic effects system has been removed, remove this dead code. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. skymp5-server/cpp/server_guest_lib/ActiveMagicEffectsMap.cpp:1
- Draft comment:
Deleted ActiveMagicEffectsMap implementation. Confirm no remaining dependency on magic effects. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. skymp5-server/cpp/server_guest_lib/ActiveMagicEffectsMap.h:1
- Draft comment:
Removed ActiveMagicEffectsMap header. Ensure calls to magic effects in other modules are updated. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. skymp5-server/cpp/server_guest_lib/AnimationSystem.cpp:15
- Draft comment:
Block event lambdas now have commented out code for modifying stamina values. Remove commented code if no longer required. - Reason this comment was not posted:
Marked as duplicate.
8. skymp5-server/cpp/server_guest_lib/MpActor.cpp:560
- Draft comment:
Removed magic effects processing functions (e.g. ReapplyMagicEffects, ApplyMagicEffect). Verify that no external code still depends on these. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. skymp5-server/cpp/server_guest_lib/MpChangeForms.cpp:43
- Draft comment:
Removed serialization of magic effects (the 'effects' field). Ensure backward compatibility with saved change forms if needed. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. skymp5-server/cpp/server_guest_lib/gamemode_events/EatItemEvent.cpp:46
- Draft comment:
Magic effects application call is commented out. Remove or replace if magic effects are no longer supported. - Reason this comment was not posted:
Marked as duplicate.
11. skymp5-server/cpp/server_guest_lib/MpActor.h:28
- Draft comment:
Typographical error: The parameter name 'calbacks_' in the constructor on line 28 appears to be a typo. Consider renaming it to 'callbacks_' for clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
12. skymp5-server/cpp/server_guest_lib/MpChangeForms.cpp:83
- Draft comment:
Consider revising the TODO comment on line 83 for clarity. Perhaps change "uncomment when add script vars save feature" to "uncomment when adding script vars save feature". - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
13. skymp5-server/cpp/server_guest_lib/MpChangeForms.h:88
- Draft comment:
Consider using 'nonexistent' instead of 'unexisting' in the comment on equipmentDump. 'Nonexistent' is more common and clear to readers. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_XUz1vstqJWk7RFad
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| animationCallbacks = { { | ||
| "blockStart", | ||
| [this](MpActor* actor) { | ||
| // constexpr float newRate = 0.f; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented-out code inside blockStart callback remains. If magic effect modifications are removed permanently, consider deleting obsolete code instead of commenting it out.
| // constexpr float newRate = 0.f; |
Important
Remove
ActiveMagicEffectsMapand related functionality fromMpActorand other components.ActiveMagicEffectsMap.cppandActiveMagicEffectsMap.h.ActiveMagicEffectsMapusage fromMpActor.cpp,MpActor.h,MpChangeForms.cpp, andMpChangeForms.h.ApplyMagicEffects,RemoveMagicEffect,RemoveAllMagicEffects, andReapplyMagicEffectsmethods fromMpActor.AnimationSystem.cpp.ApplyMagicEffectscall inEatItemEvent.cpp.This description was created by
for cfcc18e. It will automatically update as commits are pushed.