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

Cancel battle action if getting restriction state added by an event #2385

Merged
merged 1 commit into from Oct 9, 2020
Merged

Cancel battle action if getting restriction state added by an event #2385

merged 1 commit into from Oct 9, 2020

Conversation

ghost
Copy link

@ghost ghost commented Oct 9, 2020

This PR fixes #2169.

If a battler gets a restriction state added by an event his battle action gets canceled.

If a battler gets a restriction state added by an event his battle
action gets canceled.
@@ -370,6 +370,9 @@ bool Game_Battler::AddState(int state_id, bool allow_battle_states) {
if (GetSignificantRestriction() != lcf::rpg::State::Restriction_normal) {
SetIsDefending(false);
SetCharged(false);
if (GetBattleAlgorithm() != nullptr) {
this->SetBattleAlgorithm(std::make_shared<Game_BattleAlgorithm::NoMove>(this));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this is the right approach.

However, it's not clear whether this should be NoMove Null or if we just set the algo itself to nullptr. Need to test the behavior carefully in 2k and 2k3.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I tried nullptr but this approach throwed Invalid battle action error messages.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, nullptr would mean it's up to the battle system to decide what to do, which would require more changes elsewhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could give a State that autocures 100% after 1 turn to test this. When it is healed in RPG_RT then NoMove is appropriate here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using NoMove works for me in your test case.

@fmatthew5876
Copy link
Contributor

fmatthew5876 commented Oct 9, 2020

So the real difference between Null and NoMove is whether the 2k battle system will flash the actor and wait.

https://github.com/EasyRPG/Player/blob/master/src/scene_battle_rpg2k.cpp#L513
https://github.com/EasyRPG/Player/blob/master/src/scene_battle_rpg2k.cpp#L534

NoMove is like I will perform a battle action this round, which does nothing. Null literally is a placeholder for I will not perform any battle action this round.

Both of them still cause battlers to process state changes.

So in short, try to hit an enemy with a paralyze state before their turn, and then see if they flash or not. Make sure to test with no state messages displayed as messages also cause flash to occur, even for null.

@fdelapena fdelapena added this to the 0.6.3 milestone Oct 9, 2020
@Ghabry
Copy link
Member

Ghabry commented Oct 9, 2020

Oh right. I forgot that there is also a null action. Thought you mean nullptr

@ghost
Copy link
Author

ghost commented Oct 9, 2020

@fmatthew5876 I did the test with RPG_RT now and if a paralyzed enemy is hit no flash occurs with state messages disabled. In EasyRPG the paralyzed enemy flashes on its turn but that already occurred before this PR.

@fmatthew5876
Copy link
Contributor

I'm ok with accepting this PR as it is, but please write up a new issue for flashing inconsistency with 2k battles. It may be more than just this PR.

@ghost
Copy link
Author

ghost commented Oct 9, 2020

Submitted the issue now (#2388).

@fmatthew5876 fmatthew5876 merged commit 327a3bc into EasyRPG:master Oct 9, 2020
@ghost ghost deleted the issue-2169 branch October 9, 2020 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

Battle 2k: When an actor dies and is revived during the same turn the battle action is still executed
3 participants