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

Conditions in Battles #815

Merged
merged 10 commits into from Mar 14, 2016

Conversation

Projects
None yet
4 participants
@Tondorian
Member

Tondorian commented Mar 12, 2016

Conditions are applied to battle_actors now.
GainMoneyMessage only shown if money >0

I only tested it with rpg Maker 2k but should work on 2k3...

Conditions are applied to battle_actors now.
GainMoneyMessage only shown if money >0
@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Mar 12, 2016

Member

Jenkins: Test this please

Member

Ghabry commented Mar 12, 2016

Jenkins: Test this please

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Mar 12, 2016

Member

One reason speaking against ::Apply is that under RPG2k3 all actors get damage at the begin of a battle action (doesn't matter if ally or enemy). With Apply only the current one doing the action gets damage.

I suggest moving this to a function in Game_Battler and when invoking that function it directly alters SP and MP of that Battler.

For RPG2k you can place the call in ... maybe BattleActionState_ConditionHeal (for the FirstAttack case)

So for RPG2k3 I suggest placing the call to this function (and displaying the damage number (for HP only!) as a floating number) in the BattleActionState_Start for the "IsFirstAttack" case.

The RPG2k3 battle system also shows that damaging due to states happens before the action is executed.

Member

Ghabry commented Mar 12, 2016

One reason speaking against ::Apply is that under RPG2k3 all actors get damage at the begin of a battle action (doesn't matter if ally or enemy). With Apply only the current one doing the action gets damage.

I suggest moving this to a function in Game_Battler and when invoking that function it directly alters SP and MP of that Battler.

For RPG2k you can place the call in ... maybe BattleActionState_ConditionHeal (for the FirstAttack case)

So for RPG2k3 I suggest placing the call to this function (and displaying the damage number (for HP only!) as a floating number) in the BattleActionState_Start for the "IsFirstAttack" case.

The RPG2k3 battle system also shows that damaging due to states happens before the action is executed.

@Tondorian

This comment has been minimized.

Show comment
Hide comment
@Tondorian

Tondorian Mar 12, 2016

Member

git commit --amend failed again 👎

Member

Tondorian commented Mar 12, 2016

git commit --amend failed again 👎

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Mar 12, 2016

Member

git reset HEAD~1 :D should work

Because you use Windows you should consider tortoise git

Member

Ghabry commented Mar 12, 2016

git reset HEAD~1 :D should work

Because you use Windows you should consider tortoise git

Move ContitionAplliying into Game_Battler.cpp
RPG_RT2k3 should show damage numbers now
@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Mar 12, 2016

Member

Jenkins: Test this please

Member

Ghabry commented Mar 12, 2016

Jenkins: Test this please

Show outdated Hide outdated src/game_battler.cpp
Show outdated Hide outdated src/game_battler.cpp
Show outdated Hide outdated src/game_battler.cpp
Show outdated Hide outdated src/game_battler.cpp
@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Mar 12, 2016

Member

Test case for RPG Maker 2003 (add a RPG_RT.exe):
BattleTest with Monster Party 2.

rpg2k3 testcase.zip

Command Line for Player: "BattleTest 2"

Member

Ghabry commented Mar 12, 2016

Test case for RPG Maker 2003 (add a RPG_RT.exe):
BattleTest with Monster Party 2.

rpg2k3 testcase.zip

Command Line for Player: "BattleTest 2"

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Mar 12, 2016

Member

Test case for RPG Maker 2000
BattleTest with Monster Party 1.

rpg2k battletest.zip

Member

Ghabry commented Mar 12, 2016

Test case for RPG Maker 2000
BattleTest with Monster Party 1.

rpg2k battletest.zip

return false;
}
// Reset variables
battle_action_state = BattleActionState_Start;
battle_action_state = BattleActionState_ConditionHeal;

This comment has been minimized.

@Ghabry

Ghabry Mar 12, 2016

Member

You changed all the battle_action_state transitions. This is super super risky, pls don't do this unless you are 100% certain this is required. Because this must be tested with every available battle action available (Normal, different items, different skills, defend) to proof that it doesn't break anything or added a subtil bug.

One problem I already noticed is, that you introduced some delay before everybodys turn.

@Ghabry

Ghabry Mar 12, 2016

Member

You changed all the battle_action_state transitions. This is super super risky, pls don't do this unless you are 100% certain this is required. Because this must be tested with every available battle action available (Normal, different items, different skills, defend) to proof that it doesn't break anything or added a subtil bug.

One problem I already noticed is, that you introduced some delay before everybodys turn.

This comment has been minimized.

@Tondorian

Tondorian Mar 12, 2016

Member

Yeah i needed to change them to get poisen effect working before the battlers attack.
think need to double check, if waiting is needed

@Tondorian

Tondorian Mar 12, 2016

Member

Yeah i needed to change them to get poisen effect working before the battlers attack.
think need to double check, if waiting is needed

This comment has been minimized.

@Ghabry

Ghabry Mar 12, 2016

Member

I reverted all the state changes and I only took your code from line 370 to 388 and it still works fine for me.

@Ghabry

Ghabry Mar 12, 2016

Member

I reverted all the state changes and I only took your code from line 370 to 388 and it still works fine for me.

This comment has been minimized.

@Tondorian

Tondorian Mar 12, 2016

Member

so it shows conditioneffct, Clean enemyAttacks, Damage clean to you?
Takes me 2h yesterday to get it shown correct

@Tondorian

Tondorian Mar 12, 2016

Member

so it shows conditioneffct, Clean enemyAttacks, Damage clean to you?
Takes me 2h yesterday to get it shown correct

This comment has been minimized.

@Ghabry

Ghabry Mar 12, 2016

Member

In my tests your state changes broke the message box. At the beginning it shows an empty message box with a delay and between battle actions, too.

And after an enemy finished his battle animation the delay got removed.

@Ghabry

Ghabry Mar 12, 2016

Member

In my tests your state changes broke the message box. At the beginning it shows an empty message box with a delay and between battle actions, too.

And after an enemy finished his battle animation the delay got removed.

RPG2k3 everybody get damages if anyone does his turn
RPG2k Message Delay fixed
Cleanup ConditionApply() and fixed damage calculation
@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Mar 13, 2016

The damage applying is still depending on "action->GetStartSe()".

Ghabry commented on src/scene_battle_rpg2k3.cpp in 4e501f6 Mar 13, 2016

The damage applying is still depending on "action->GetStartSe()".

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Mar 13, 2016

Member

The delay is still wrong here.

Normal attacks without any states have a 30 frame delay before anything happens -> Must be 0.

And after battle animations the feedback is now instant instead of a delay. (After showing the attack animation, e.g. the sword slash, the old version had a 30 frame delay).

And sometimes enemy attacks give me now a negative amount and heal me :O

Member

Ghabry commented Mar 13, 2016

The delay is still wrong here.

Normal attacks without any states have a 30 frame delay before anything happens -> Must be 0.

And after battle animations the feedback is now instant instead of a delay. (After showing the attack animation, e.g. the sword slash, the old version had a 30 frame delay).

And sometimes enemy attacks give me now a negative amount and heal me :O

@Tondorian

This comment has been minimized.

Show comment
Hide comment
@Tondorian

Tondorian Mar 13, 2016

Member

I think it should work as expected now
if you can confirm, i'll clean up my branch

Member

Tondorian commented Mar 13, 2016

I think it should work as expected now
if you can confirm, i'll clean up my branch

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Mar 13, 2016

Member

I will prepare an emscripten test case for both battle systems this evening.
Then it's easier to verify this (also for future pull requests).

But I think you are getting closer now :D

Member

Ghabry commented Mar 13, 2016

I will prepare an emscripten test case for both battle systems this evening.
Then it's easier to verify this (also for future pull requests).

But I think you are getting closer now :D

@Ghabry Ghabry added this to the 0.4.1 milestone Mar 13, 2016

@carstene1ns

This comment has been minimized.

Show comment
Hide comment
@carstene1ns
Member

carstene1ns commented Mar 13, 2016

added missing include
fix another timing error
@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Mar 14, 2016

Member

Jenkins: Test this please

@fdelapena Maybe we should whitelist him ;)

Member

Ghabry commented Mar 14, 2016

Jenkins: Test this please

@fdelapena Maybe we should whitelist him ;)

@Tondorian

This comment has been minimized.

Show comment
Hide comment
@Tondorian

Tondorian Mar 14, 2016

Member

And sometimes enemy attacks give me now a negative amount and heal me :O

See this on your Emscripten Testcase first time
I think i did not change anything on this part game_battlealgoithm.cpp

Member

Tondorian commented Mar 14, 2016

And sometimes enemy attacks give me now a negative amount and heal me :O

See this on your Emscripten Testcase first time
I think i did not change anything on this part game_battlealgoithm.cpp

@fdelapena

This comment has been minimized.

Show comment
Hide comment
@fdelapena

fdelapena Mar 14, 2016

Contributor

@fdelapena Maybe we should whitelist him ;)

Done!

I think i did not change anything on this part game_battlealgoithm.cpp

Yeah, Ghabry commented on IRC this also happens with the current upstream master version.

Contributor

fdelapena commented Mar 14, 2016

@fdelapena Maybe we should whitelist him ;)

Done!

I think i did not change anything on this part game_battlealgoithm.cpp

Yeah, Ghabry commented on IRC this also happens with the current upstream master version.

@Tondorian

This comment has been minimized.

Show comment
Hide comment
@Tondorian

Tondorian Mar 14, 2016

Member

but i messed up dead enemys in rpg2k3 :D if they are dead they get 1hp ^^
will fix this part now

Member

Tondorian commented Mar 14, 2016

but i messed up dead enemys in rpg2k3 :D if they are dead they get 1hp ^^
will fix this part now

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Mar 14, 2016

Member

Okay, looks fine now. The negative HP after attacks and poison are strange but this already happens in Master... :(.
@Tondorian Is the PR ready?

Member

Ghabry commented Mar 14, 2016

Okay, looks fine now. The negative HP after attacks and poison are strange but this already happens in Master... :(.
@Tondorian Is the PR ready?

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Mar 14, 2016

This change is not required. When effect is set to 0, the change equation will be 0, too.

Ghabry commented on src/game_battlealgorithm.cpp in a9fdd28 Mar 14, 2016

This change is not required. When effect is set to 0, the change equation will be 0, too.

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Mar 14, 2016

Member

The problem for that "-5" is the skill damage code in Game_BattleAlgorithm::Skill::Execute()

We need a "effect < 0 -> = 0" check before the line with rand().

Member

Ghabry commented Mar 14, 2016

The problem for that "-5" is the skill damage code in Game_BattleAlgorithm::Skill::Execute()

We need a "effect < 0 -> = 0" check before the line with rand().

Ghabry added a commit that referenced this pull request Mar 14, 2016

@Ghabry Ghabry merged commit 118e900 into EasyRPG:master Mar 14, 2016

5 checks passed

Android Build finished.
Details
Linux Build finished.
Details
OSX Build finished.
Details
Windows Build finished.
Details
web Build finished.
Details

@Tondorian Tondorian deleted the Tondorian:mergeRequest branch Mar 14, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment