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

Refactor Battle2k console and action state machine #1573

Merged
merged 23 commits into from Jan 20, 2019

Conversation

Projects
None yet
4 participants
@fmatthew5876
Copy link
Contributor

fmatthew5876 commented Dec 16, 2018

Fixes: #1570

As promised after battle 7 was merged, this is a series of commits to refactor the battle console and cleanup some of the mess from battle 5.

Currently this uses gtest for unit testing the WordWrap() function. I will change it to doctest, but for now I've put this PR up for early feedback.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:battle_console branch from 0d278d3 to e307071 Dec 16, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Dec 16, 2018

So next task will be supporting word wrap properly.
I've got a skill with usage message 1 as 1%S%S%S%S 2%S%S%S%S 3%S%S%S%S%S 4%S%S%S%S 5%S%S%S 6%S%S%S%S 7%S%S%S%S and usage message 2 as 11111111112222222222333333333344444444445555555555

Here is what steam RPG_RT does with it:

output

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Dec 16, 2018

Here are some test cases for the start of battle

The start message is %S XXXXXXXXXXXXXXXXXXXX YYYYYYYYYY %S %S

output-start
output-start2
output-start3

The start message is App 0%S%S 1%S%S 2%S%S 3%S%S 4%S%S 5%S%S 6%S%S 7 %S%S 8%S%S

start3

The start message is App 0%S%S%S 1%S%S%S 2%S%S%S 3%S%S%S 4%S%S%S 5%S%S%S 6%S%S%S 7%S%S%S 8%S%S%S

start4

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:battle_console branch 5 times, most recently from ffee361 to c0ef41e Dec 16, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Dec 16, 2018

Test cases for usage messages:

Skill Using Messages and Animation Timing. Animation starts on the last of the 2 usage messages:

usage1

Skill with no usage message:

usage0

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:battle_console branch 2 times, most recently from 336db78 to eb30c24 Dec 16, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Dec 16, 2018

Test cases for weapons:

A 2 weapon attack. The animations used are Water Magic One A and Fire Magic One B

RPG_RT is has a bug and plays them overtop of each other. I'm testing in wine so I think this is a wine bug. I don't remember this behavior and my game relies heavily on this not happening.

2attack

A weapon with a long attack animation that crits. Critical hit message wait for animation to complete.
2attack2

A weapon with attack all that crits and kills

attack-all1

A weapon with double attack which crits and kills
2attk

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:battle_console branch 2 times, most recently from 99fc27a to eddf5c7 Dec 16, 2018

@fmatthew5876 fmatthew5876 changed the title Refactor the battle console - WIP Refactor Battle2k console and action state machine Dec 16, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:battle_console branch 3 times, most recently from 5ab6a27 to 897b4c6 Dec 16, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Dec 16, 2018

Escape and Self-Destruct:

escape
explode

Skill that inflicts death and has no usage message:
death

A double attack weapon that inflicts poison. The first hit inflicts the state, the second hit would also inflict the state, but the target already has it. For skills, when the target has the state, we always see the "already inflicted" message, but for weapons we don't see any message.
poison

A skill which has 0% chance to inflict poison on enemies who are already poisoned.
poison2

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:battle_console branch 4 times, most recently from 8fcc154 to 1e1ef76 Dec 17, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Dec 17, 2018

Now we need to figure out the testing framework question?

What is the best way to pull in doctest? I see we have a copy in liblcf. Are we going to base everything on the liblcf one? Or change everything to use upstream version?

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:battle_console branch 4 times, most recently from 7117a1d to d21d155 Dec 17, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Dec 17, 2018

This is ready for review and testing. The timings of the console delays are not perfect. We need a frame accurate video capture to tweak it.

Show resolved Hide resolved src/scene_battle_rpg2k.cpp Outdated
@carstene1ns
Copy link
Member

carstene1ns left a comment

Just style nits:

  • Comment style: //[space]comment
Show resolved Hide resolved src/game_battlealgorithm.h
Show resolved Hide resolved src/window_battlemessage.cpp Outdated
Show resolved Hide resolved tests/CMakeLists.txt Outdated

fmatthew5876 and others added some commits Dec 15, 2018

Refactor wordwrap
* Fixes hang when string.size() > 50 characters and has no spaces.
* Fixes width used when word wrap is called. Current one cuts too early.
* Unit tests for different strings
* Graphical adjustments to ensure long lines get rendered properly. The
  standard RPG_RT fonts use 50 characters. Previously we would render
  half of the 51st character as the drawing bitmap was not bounded
  correctly.
Refactor Window_BattleMessage
* Correctly prints word wrapped messages to the console
* No delays for word wrapped lines
* Breaks header messages like usage, criticals, etc..
Refactor battle 2k start enemy display
* Use new Window_BattleMessage API
* Cancel key freezes the console while its held down
* Match RPG_RT behavior for timings and word wrap
Refactor Action ConditionHeal
* Move into its own function
* Bugfix: Don't flash hidden enemies
* Bugfix: Fix delay on condition messages
Separate battle action states for 2k and 2k3.
These describe the flow of logic in each battle system. Should
be customized to each and not shared between them.
Fix behavior when weapon inflicts state target already has
Prints and does nothing in this case. Unlike skills.
Fix Normal attack variance
Variance should be applied at the end of dmg calc
UpdateBattlerActions() -> PrepareBattleAction()
Simplified this logic. Also fixes a bug where enemies who only
do NoMove action would not attack the enemy party when confused
Fix attribute shift from skills
Attributes would not be attempted unless skill failed before??
Makes no sense, should always try attributes
Remove IsBattleAnimationOnlySound
This doesn't affect how we wait
Update src/game_battlealgorithm.h comment
Co-Authored-By: fmatthew5876 <fmatthew5876@gmail.com>

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:battle_console branch from 98d1601 to 085feba Jan 5, 2019

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Jan 5, 2019

I've addressed @carstene1ns review comments and rebased to master

@carstene1ns carstene1ns merged commit 91be195 into EasyRPG:master Jan 20, 2019

7 checks passed

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

This comment has been minimized.

Copy link
Member

Ghabry commented Jan 27, 2019

There is no way to [Approve] after a merge 👎

@fmatthew5876 fmatthew5876 deleted the fmatthew5876:battle_console branch Jan 29, 2019

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.