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

Effects which can be removed by a medikit are no longer removed by revival protocol #1438

Merged

Conversation

BlackDog86
Copy link
Contributor

Fixes #1435

@BlackDog86 BlackDog86 requested a review from Iridar January 1, 2025 19:40
@BlackDog86 BlackDog86 self-assigned this Jan 1, 2025
@BlackDog86 BlackDog86 added this to the 1.30.0 milestone Jan 1, 2025
@BlackDog86 BlackDog86 force-pushed the 1435-Further-Revival-Protocol-Fixes branch from f0f8a73 to 3af8f8a Compare January 2, 2025 12:12
@Iridar
Copy link
Contributor

Iridar commented Jan 4, 2025

As much as this fix makes sense, backwards compatibility policy prevents us from altering the existing mod-facing functions. I suggest making a new RemoveAdditionalEffectsForRevivalProtocolAndRestorativeMist_CH() function and calling it instead where relevant. This would fix Revival Protocol without affecting any other abilities that might be using the original RemoveAdditionalEffectsForRevivalProtocolAndRestorativeMist(), whether that change would be beneficial or not.

@Iridar Iridar added the waiting-on-author A pull request is waiting on changes from the author label Jan 4, 2025
@BlackDog86
Copy link
Contributor Author

Yeah, I had considered making it work that way initially. I think the original function is only called twice in the specialist ability set anyway so it's a straightforward alteration.

@BlackDog86 BlackDog86 force-pushed the 1435-Further-Revival-Protocol-Fixes branch 2 times, most recently from d250783 to dfd5002 Compare January 4, 2025 11:25
@BlackDog86
Copy link
Contributor Author

Adjusted the commit with a new function for BC. Actually works out better because restoration can continue to use the old function which does heal those things, and we can just adjust this for revival protocol.

@BlackDog86 BlackDog86 added ready-to-review A pull request is ready to be reviewed and removed waiting-on-author A pull request is waiting on changes from the author labels Jan 4, 2025
@BlackDog86 BlackDog86 force-pushed the 1435-Further-Revival-Protocol-Fixes branch from dfd5002 to f0f794b Compare January 4, 2025 11:31
Renamed new function and adjusted docs.
@Iridar Iridar added ready-for-merge the pull request was reviewed and is ready to be merged. and removed ready-to-review A pull request is ready to be reviewed labels Jan 4, 2025
@Iridar Iridar merged commit b4bcf48 into X2CommunityCore:master Jan 6, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-basegame ready-for-merge the pull request was reviewed and is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revival Protocol cures the same statuses that Medikit does, if able
2 participants