Skip to content
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

Sync Remove Spell / Magic Cancel #735

Merged
merged 3 commits into from
Nov 22, 2024

Conversation

rfortier
Copy link
Contributor

I'm just the PR monkey, this fix is contributed by @MostExcellent.

This allows the function that explicitly removes magic effects to sync when applied to a player. The Remove Spell function is in vanilla Skyrim but won't be synced by STR without this fix.

Updated to newest newest /dev branch, needed to move Actor::RemoveSpell to Actor.cpp

This functionality has been in the Animation branch since before the release of 1.6.7, just got the author's permission to PR/merge it.

This allows the function that explicitly removes magic effects to sync when applied to a player
Cherry-pick to newest /dev branch needed to move Actor::RemoveSpell to Actor.cpp
@RobbeBryssinck
Copy link
Member

I'm not sure what this is supposed to do. Can you give an example of what bug this would fix for example? For example, are magic effects not cancelled properly without this PR, and if so, which ones?

@rfortier
Copy link
Contributor Author

rfortier commented Nov 15, 2024

I'm not sure what this is supposed to do. Can you give an example of what bug this would fix for example? For example, are magic effects not cancelled properly without this PR, and if so, which ones?

Suspected I would get this question. But I don't know where vanilla Skyrim uses it. I guess I can try a breakpoint and see if it fires with the way I play. MCO players discovered it because some effects in MCO weren't working/syncing right, and found it was using this vanilla Skyrim function that wasn't being synced.

Well, so I did that. The first thing I tried called RemoveSpell, changing my standing stone blessing. So it must happen a lot in the vanilla game. Understanding all the implications is a lot harder, of course.

This was my pure-vanilla, no-mods game.

@marceloarc
Copy link

marceloarc commented Nov 18, 2024

This pr fixed an annoying bug where healing hands was not working very well, i'm sure this fixed other bugs as well

Code/client/Games/Skyrim/Actor.cpp Outdated Show resolved Hide resolved
Code/client/Games/Skyrim/Magic/MagicTarget.cpp Outdated Show resolved Hide resolved
Code/encoding/Messages/RemoveSpellRequest.h Outdated Show resolved Hide resolved
@rfortier
Copy link
Contributor Author

This is the first time I implemented an It() / HookIt() / RealIt() pattern, so I pushed the branch early for comment on whether I got all the parts in the right places.

I'll resolve the review comments after I finish testing.

@rfortier
Copy link
Contributor Author

Changes are in and tested to the extent I can. I recommend getting an MCO user to verify, or someone who knows of a reliable vanilla test.

@RobbeBryssinck RobbeBryssinck merged commit 16c3786 into tiltedphoques:dev Nov 22, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants