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

Albeleon's Battle Fixes 6 of 7 #1451

Merged
merged 14 commits into from Dec 6, 2018

Conversation

Projects
None yet
5 participants
@fmatthew5876
Copy link
Contributor

fmatthew5876 commented Oct 15, 2018

No description provided.

@fmatthew5876 fmatthew5876 reopened this Nov 5, 2018

@fmatthew5876 fmatthew5876 changed the title Albeleon's Battle Fixes 5 of N Albeleon's Battle Fixes 6 of N Nov 5, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:albeleon_battle_5 branch from 0ce98f1 to a09956b Nov 5, 2018

@fdelapena fdelapena added Needs Rebase and removed Needs Rebase labels Nov 5, 2018

@fmatthew5876 fmatthew5876 reopened this Nov 29, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:albeleon_battle_5 branch from a09956b to 995d2b4 Nov 29, 2018

@fmatthew5876 fmatthew5876 changed the title Albeleon's Battle Fixes 6 of N Albeleon's Battle Fixes 6 of 7 Nov 29, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Nov 29, 2018

All commits that show up in this PR have been reviewed, tested, and approved by me. Some are new.

Changed:

  • 2.22 - Incorrect algorithm for choosing message

How I tested:

  • 2.14 - Set your actor to 1 out of 1 criticals and attack
  • 2.18 - Make a state that has a "state still active" message. Apply the state to enemies with a battle event. Watch the timing of the message and enemy flash.
  • 2.22 - Apply 2 states with a battle event. Test all combinations of priorities (include same), heal vs affected, and empty message behavior.
  • 2.32 - Test using items on dead characters and k.o. only items on live characters.
  • Fix appearance ... - Test recovery medicine, skills, and skills from items on characters with full HP/SP
  • Correctly handle empty start... - Use a skill with no usage message that heals both HP and MP.
  • 2.29 - Make a skill that boosts attributes, make another that reduces them.
  • Encapsulate effects of death - Test skills and items which recover death and verify HP matches RPG_RT. Test skills which cause death. Test death from damage.
  • Don't print HP recovered - Test skill/item which revive and have "affect hp" checked
  • 2.37 - Test skill which revive, has effect > 0, and "affect hp" not checked.
  • 3.11 - Try healing and revive skills

@fdelapena fdelapena added Battle and removed Needs Rebase labels Nov 29, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:albeleon_battle_5 branch 5 times, most recently from 4595992 to 24f6a03 Nov 29, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:albeleon_battle_5 branch 5 times, most recently from 73c8b7c to b3143cb Dec 2, 2018

@Ghabry
Copy link
Member

Ghabry left a comment

all comments belong to that "print attrib resistance" commit

Show resolved Hide resolved src/game_battler.h Outdated
Show resolved Hide resolved src/game_battlealgorithm.h Outdated
Show resolved Hide resolved src/game_battlealgorithm.cpp Outdated
Show resolved Hide resolved src/game_battlealgorithm.cpp Outdated
Show resolved Hide resolved src/game_battlealgorithm.cpp Outdated

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:albeleon_battle_5 branch 2 times, most recently from 052a4a4 to 43105cc Dec 3, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Dec 4, 2018

I fixed 2.22 and did the requested refactors for 2.29.

I've retested 2.29 and all the behavior works. It will still need to be reviewed again to ensure no rebase mistakes.

I added the CanShiftAttribute() method, but in general I don't like this approach of check if we can, set success state, and then apply the change. We basically have to duplicate our logic twice all the time.

I'd rather just the apply returned a bool and then we set success at the end if anything happened. I will look into such a refactor after battle 7 is done.

Show resolved Hide resolved src/game_battlealgorithm.cpp Outdated

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:albeleon_battle_5 branch 3 times, most recently from 8d34476 to ea2d7df Dec 5, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:albeleon_battle_5 branch from ea2d7df to 4f121dc Dec 5, 2018

Albeleon and others added some commits Jun 23, 2018

Big commit: Issues fixed: 2.2 + 3.1 + 3.2
Big change: The structure of ProcessBattleActions. The way it was before, the enum was: "Start, ConditionHeal, Result, Finished". The order in which they were executed were "ConditionHeal > Start > (Result) > Finished".

Now, the new enum and order (also reflected in the order in ProcessBattleActions) is this (names also changed in scene_battle_rpg2k3.cpp):
1-ConditionHeal: Pretty much the same.
2-Execute: This is the Start function, but only does the action->Execute(), get the result messages, and print the first start message.
3-Apply: This is the next part of Start function. It prints the second start message if there is one; it executes the sounds and animations; and does Action->Apply.
4-ResultPop: This result simply pops the right number of messages and keep the ones that should be left (minimum the start messages).
5-ResultPush: This result prints and push each new result message. In case it's the first message, it does the proper sound, shake and enemy animation effects. It adds one to the pointer of the next message, and if it didn't finish, it goes back to ResultPop. If it finishes, it goes to the next BattleActionState.
6-Death: This generates the death animation if it's the case and waits GetDelayForWindow().
7-Finished: This concludes the action. It waits until the damaged animation of an enemy is finished, resets the BattleAction_State to ConditionHeal, and returns true or false depending of whether the attack should be made to the next enemy.

Reasons why I made this:
-By ordering them and separating their functions, the code is more legible and procedural, without confusing iterations inside the same State full of branches.
-ResultPop waits GetDelayForLine() / 2, which generates the flash in the issue 3.2.
-Death and Finished are separated because in Death there is a GetDelayForWindow() time before it goes to finish and has the option to return true.

Other things:

-Some states end with invoking the function recursively. This is because, in case the action is NoMove, we want it to execute the fastest, and because there are so many States, if it waits one frame for each one, the delay would be noticeable, it should be as fast as possible. It also helps skip the first steps (Execute, Apply) if it's not the first target of an attack.

-Battle_Result_Order: One of the previous problems was that, some result messages are push without overriding the previous result message (i.e. when a condition is healed by a physical attack, or when an enemy dies). To fix this, now Game_BattleAlgorithm::GetResultMessages returns, aside of a vector with messages, a vector with int called "Battle_result_order". Each time a result message is created, it's also pushed a number that sets the position or priority. This means, if the priority is 0, this will always be printed right after the start messages and overriding the rest (they are popped by ResultPop). If the priority is 1, the first result message is not popped, and the new message appears after that. This allows us to generate result messages without overriding the previous one in some cases. In most cases is 0, but in the previous examples it is 1.

Overall this fixes the issue 3.1 (messages rightly printed), 2.2 (inform about the healed conditions) and 3.2 (a few flash before a new result message is printed). I hope the structure is easy to understand and you like it despite how confusing the commit by itself might be.
"2.14: RM2000: Critical hits from attacks should be displayed."
Solution: A critical hit is printed before the damage at GetDelayForLine() * 2 speed, and it's always kept for the rest of the Battle action to that target. Therefore, we need to make a few modifications:
1. Create a local variable "critical_hit".
2. In ResultPop and ResultPush, when the action has been executed and we can calculate the value, we make "critical_hit" = 1 if the attack succeeded and it's critical. Otherwise, 0.
3. Since the critical hit message will be kept, that means the ResultPop should keep in mind whether there is a critical message or not. So, in the "while() pop" function, we add this value so it doesn't pop the critical_hit message.
4. Since the first message to be printed can be critical_hit before the HP, we should add the critical_hit value to the execution of the effects (enemy animation, screen shake, sound...), so it's only done in the second iteration, not the first one.
5. After the critical_hit message has been pushed and the result pointer has been increased, we check if the first message was a critical hit right after. If it is, the battle_action_wait, instead of GetDelayForWindow(), it will be GetDelayForLine() * 2.
"2.18: RM2000: The enemy flash that indicates it's their turn, it onl…
…y appears once, and it appears before the condition_heal, not when it starts their action."

Solution: Move the flash to ConditionHeal, concretely inside "IsFirstAttack".
"2.22: RM2000: Correct printing of state heal/affect messages
* Of all states, only 1 message is ever printed and only for the most
  significant state.
* Choose most significant state of union of inflicted states
  and healed this round states.
* If was healed, print and wait for the healed message even if empty
* Otherwise print affected message if not empty

Replaced original Albeleon 2.22 commit
"2.32: RM2000 + RM2003: When throwing an item to a dead character wit…
…hout the "only k.o." option, the target is still the dead person, it doesn't change, and the attack can succeed (it can still recover SP and states like Dead for example and resurrect, even if no HP will be taken if it doesn't cure Death). An item fails if "only k.o." is checked and it's thrown to an alive actor, or if it doesn't recover any HP, SP or condition."

Solution:

1. Since in items the target is always valid, we take away the condition that checks medicine during death.
2. We put, in case the "ko_only" attribute is active but the target is alive, that the item misses.
3. We add that a condition to heal is only added if the character suffers that condition. Otherwise, it skips it.
4. We check if the character is dead and the medicine doesn't cure Death. If that's the case, the HP restored will be resetted to -1.
5. In case there's no HP, SP or condition, the attack fails.

We also solve a few things in AlgorithmBase::Apply to ensure consistency:

1. If a character is Dead, then it doesn't restore health.
2. If a character is resurrected, then it restores the amount of HP. If it's 0, then it will restore 1.
Fix appearance of HP/SP recovery message
* Skill: Recovery messages always appear
* Item: Only appear if HP/SP recovered > 0
* Item that triggers skill: Same as skill
* Does not need to check death state as other code must ensure no hp healed.
Correctly handle empty start message
When start message is empty, we still delay for it
but we don't display a line above the affects.
"2.29: RM2000: Print "attribute resistance / weakness" result message…
… in skills when it's successful, and make sure if it's active that its increase/decrease is given the same success features as a state (if the parameters fail in a way that ignores the state, it ignores this too; if it cannot provoke any attribute UP/DOWN effect, it doesn't turn the attack in a success)."

Solution:

1. Add in Game_BattleAlgorithm a "shift_attributes" variable.
1. In Skill::Execute, after the state/condition (so if parameters force the end of the operation, this will be too), check if an attribute resistance can be increased or decreased. If it can, and the to_hit is a success, then push the attribute id.
2. Print in Skill::GetResultMessages, after the other result messages. To print the right message, we create a function GetShiftAttributeMessage, taking as input whether the skill is positive or not (UP/DOWN) and the attribute's name. It will be printed the right way for each RPG Maker. *
3. We apply it in Skill::Apply.
4. Since there's no function to check whether a new ShiftAttributeRate will change something or not, a GetShiftAttributeRate function has been added to Game_Battler, which returns the current modifier for that attribute in that battler (-1, 0 or 1).

* IMPORTANT: Since I don't know japanese, I don't know if the japanese particles written here are the correct ones. Could someone verify?
"2.37: RM2000 + RM2003: If a skill resurrects a character but doesn't…
… have the "affect_hp" checkbox, the effect value will restore the character's %HP (100 or more will restore the character completely). If other stat checkbox are selected, they still gain the same amount of absolute effect value."

Solution: If "affect_hp" isn't put and the condition restores Death, then change the HP to cure the percentage of the target's HP. Since we need the conditions saved to know whether it cured Death or not, this has to go after conditions (way after the parameters). So we need to preserve the "effect" value, which is why the local variable "effect" has been put at the beginning of the function.
Encapsulate effects of death
External code should be able to just add and remove
death state.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:albeleon_battle_5 branch from 263a3eb to f8ac43b Dec 6, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Dec 6, 2018

Rebased to master

@Ghabry

Ghabry approved these changes Dec 6, 2018

@Ghabry Ghabry merged commit 309727a into EasyRPG:master Dec 6, 2018

6 checks passed

Android (armeabi-v7a) Build finished.
Details
GNU/Linux Build finished.
Details
OSX Build finished.
Details
Windows (x64) Build finished.
Details
Windows (x86) Build finished.
Details
web Build finished.
Details

@fmatthew5876 fmatthew5876 deleted the fmatthew5876:albeleon_battle_5 branch Dec 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.