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

RPG Maker 2003 battle scene changes #2483

Merged
merged 14 commits into from Apr 17, 2021
Merged

RPG Maker 2003 battle scene changes #2483

merged 14 commits into from Apr 17, 2021

Conversation

ghost
Copy link

@ghost ghost commented Mar 15, 2021

This PR does the following changes:

  • The status window components have been rearranged to match the RPG_RT style.
  • The transparent windows use an alpha of 160 instead of 128 now (tested with truecolor RPG_RT).
  • In traditional style battles two bugs have been fixed: The SP window got printed over when a skill which costs SP has been used and the HP value didn't change its color if the HP reached critical or zero.
  • In alternative style battles the gauges are now shown transparent if the window is opaque.
  • The ally cursor is always drawn 40 pixels above the Y-Position of the battler.
  • The enemy cursor is always drawn on the same height as the Y-Position of the enemy.
  • Flipped actors use now the correct walking animation on escaping.
  • The battle status window gets refreshed on applying state effects. This stops the HP value "lagging" behind on traditional battle mode.
  • The enemy target window gets refreshed as soon as an enemy dies.

@Ghabry Ghabry added Battle Has PR Dependencies This PR depends on another PR labels Mar 15, 2021
@ghost ghost mentioned this pull request Mar 15, 2021
91 tasks
@Ghabry Ghabry added Awaiting Rebase Pull requests with conflicting files due to former merge and removed Has PR Dependencies This PR depends on another PR labels Mar 21, 2021
@Ghabry Ghabry added this to the 0.6.3 milestone Mar 21, 2021
@ghost ghost marked this pull request as ready for review March 22, 2021 04:55
@fdelapena fdelapena added Window/Scenes and removed Awaiting Rebase Pull requests with conflicting files due to former merge labels Mar 22, 2021
contents->StretchBlit(Rect(x, y, 13, 16), *system2, Rect(48 + gauge_x, 32 + 16 * which, 8, 16), Opacity::Opaque());
contents->StretchBlit(Rect(x + 13, y, gauge_width - 13, 16), *system2, Rect(56 + gauge_x, 32 + 16 * which, 8, 16), Opacity::Opaque());
} else {
contents->StretchBlit(Rect(x, y, gauge_width, 16), *system2, Rect(48 + gauge_x, 32 + 16 * which, 8, 16), Opacity::Opaque());
Copy link
Member

Choose a reason for hiding this comment

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

Battle 2k3: Change gauge display

Can you explain what the magic number 13 means here?
Also for what is the pixman expert?

Copy link
Author

Choose a reason for hiding this comment

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

I admit this one is more of a hack. I take the left 8 pixels of the gauge and stretch them to 13 pixels, and the right 8 pixels of the gauge gets stretched to 12 pixels, which sums to 25 pixels like in RPG_RT. By putting the two halves together you get a more accurate output than directly stretch the entire gauge from 16 to 25 pixels. The so-called "pixman expert" is somebody we need to refactor the StretchBlit and similar functions to match RPG_RT behavior (see issue #446). As far as I know we are using pixman for the pictures, so I went with the term "pixman expert" for people who are experienced in using this library. (This entire stuff here is kinda hard to explain...)

Copy link
Member

Choose a reason for hiding this comment

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

Can I suggest something?
Could you try to replicate a RPG2k3 gauge using operations (stretch) in a normal image editor like Gimp or Paint.net?
When the exact operations are known this is easier to replicate in the Player instead of experimenting.

Copy link
Author

Choose a reason for hiding this comment

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

Tested with GIMP and the scale operation there unfortunately does not match the StretchBlit function.

Copy link
Member

Choose a reason for hiding this comment

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

But with gimp you get the result you want?

Copy link
Author

Choose a reason for hiding this comment

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

No.

Copy link
Author

Choose a reason for hiding this comment

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

Alternatively we can remove the gauge stuff from this PR and wait until #446 is fixed (this should automatically fix the gauge display).

Copy link
Member

Choose a reason for hiding this comment

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

Okay. Please remove this commit then. As this is not a 100% fix I do not think that making the code harder to read is worth it here. Sorry

Copy link
Author

Choose a reason for hiding this comment

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

Removed the gauge commit now.

@@ -155,7 +159,9 @@ void Window_BattleStatus::RefreshGauge() {
int y = 2 + i * 16;

if (lcf::Data::battlecommands.battle_type == lcf::rpg::BattleCommands::BattleType_alternative) {
DrawGauge(*actor, 202 - 10, y - 2);
if (lcf::Data::battlecommands.transparency == lcf::rpg::BattleCommands::Transparency_opaque || (2 + index * 16 != y)) {
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of the 2 + index * 16 != y check?

Copy link
Author

Choose a reason for hiding this comment

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

This check stops the gauge (if the gauge is opaque) being drawn if the menu cursor is on the same position.

Copy link
Member

@Ghabry Ghabry Apr 12, 2021

Choose a reason for hiding this comment

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

I thought about this now for 15 minutes but now I fully understand why they did this (typical 2k3 half assed job):

The gauge is drawn transparent but above the selection box. This means that the color of the gauge would be wrong because of the selection box as the draw order is "Background, Selection Box, Gauge, Text".

Instead of thinking about a solution they simply hid the bug. Genious move -_-.

Could you add a comment there:

RPG_RT Bug (?): Gauge hidden when selected due to transparency (wrong color when rendering)

Or something similiar.

Copy link
Author

Choose a reason for hiding this comment

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

Added.

@@ -508,6 +508,7 @@ void Scene_Battle_Rpg2k3::RefreshTargetWindow() {
// FIXME: Handle live refresh in traditional when the window is always visible
auto commands = GetEnemyTargetNames();
target_window->ReplaceCommands(std::move(commands));
if (!target_window->GetActive()) target_window->SetIndex(-1);
Copy link
Member

Choose a reason for hiding this comment

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

Missing {} and indent.

Can your change cause any race condition? E.g. the target window is open and while open an enemy dies (if this is even possible).
Does this fix any known bug?

Copy link
Author

Choose a reason for hiding this comment

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

Fixed the missing {} and indent. This change fixes the cursor being shown even if the target window is not active. In RPG_RT the cursor is only shown in the target window if you are selecting a target. If the target window is open the active flag should always be set which means that the case you mentioned should never occur.

@@ -88,7 +88,7 @@ void Window_BattleStatus::Refresh() {
} else {
if (lcf::Data::battlecommands.battle_type == lcf::rpg::BattleCommands::BattleType_traditional) {
DrawActorState(*actor, 84, y);
contents->TextDraw(136 + 4 * 6, y, Font::ColorDefault, std::to_string(actor->GetHp()), Text::AlignRight);
contents->TextDraw(136 + 4 * 6, y, actor->GetHp() == 0 ? Font::ColorKnockout : actor->GetHp() <= actor->GetMaxHp() / 4 ? Font::ColorCritical : Font::ColorDefault, std::to_string(actor->GetHp()), Text::AlignRight);
Copy link
Member

Choose a reason for hiding this comment

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

I agree with c1 that this ternary is hard to read.
https://github.com/EasyRPG/Player/blob/master/src/window_base.cpp#L177-L181

Copy link
Member

Choose a reason for hiding this comment

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

We have this in 3 places now at least. Maybe is time to make this a real function.

Copy link
Member

Choose a reason for hiding this comment

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

yeah.
Maybe add two new functions to window_base.h which simply draw the currect HP/MP value at cx/cy. There is already DrawActorHp but this one is not generic enough for this.

void DrawActorHpValue(const Game_Battler& actor, int cx, int cy) const;
void DrawActorSpValue(const Game_Battler& actor, int cx, int cy) const;

Copy link
Author

Choose a reason for hiding this comment

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

Added the functions and replaced the ternary.

Copy link
Member

Choose a reason for hiding this comment

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

The color logic is still duplicated though.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed (hopefully).

Copy link
Member

Choose a reason for hiding this comment

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

yes looks good

@@ -301,7 +301,7 @@ void Scene_Battle_Rpg2k3::CreateUi() {
}

if (lcf::Data::battlecommands.battle_type != lcf::rpg::BattleCommands::BattleType_traditional) {
int transp = lcf::Data::battlecommands.transparency == lcf::rpg::BattleCommands::Transparency_transparent ? 128 : 255;
int transp = lcf::Data::battlecommands.transparency == lcf::rpg::BattleCommands::Transparency_transparent ? 160 : 255;
Copy link
Member

Choose a reason for hiding this comment

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

btw. this is also referenced in multiple places, so maybe make this a function as well? …IsTransparent()
https://github.com/EasyRPG/Player/search?q=battlecommands.transparency

Copy link
Member

Choose a reason for hiding this comment

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

good idea, as this is 2k3 only feature the `IsTransparent`` func can be in the 2k3 battle scene directly.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

@Ghabry
Copy link
Member

Ghabry commented Apr 12, 2021

Have to check the following if the pixels match:

  • Battle 2k3: Rearrange components in the battler status window (1/2)
  • Battle 2k3: Rearrange components in the battler status window (2/2)
    But rest looks already good 👍

rueter37 added 14 commits April 13, 2021 17:31
If the battle windows are set to transparent, then use an alpha of
160 for the battle windows.
The states, HP values, SP values and gauges have been rearranged in
the battler status window in the alternative battle type.
The states, HP values, SP values and gauges have been rearranged in
the battler status window in the traditional battle type.
The HP values did not display the colors for dead or critical HP in
traditional type battles. This is fixed now.
The SP window in the traditional battle type was not cleared when the
text had changed which led to overlapped texts. This is fixed now.
If the battle mode is alternative and the window is set to opaque,
then display the gauges with an alpha of 96.
The ally cursor is now always shown 40 pixels above the Y-Position of
the battler.
The enemy cursor is now always shown equal to the Y-Position of the
enemy.
Actors always use the WalkingRight animation on escape now.
The battle status window is refreshed now when state effects are
getting applied.
The enemy target window is now updated on enemy death.
The IsTransparent function has been added now. It is used in the
RPG Maker 2003 battle scene to check if the transparency flag in
LCF database is set. This is used for the windows in the battle
scene.
The new DrawActorHpValue and DrawActorSpValue functions draws the HP
or SP value without the stat name and maximum value. This is used now
in RPG Maker 2003 traditional battle mode where only the HP value
(without maximum) is shown in the battle window.
The function GetValueFontColor has been added now. This returns a
color value depending on the value in relation to the maximum
value.
Copy link
Member

@Ghabry Ghabry left a comment

Choose a reason for hiding this comment

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

Because this is pure visual I give a 7 day grace period for reviews.

@fmatthew5876 fmatthew5876 merged commit 7ddc8ea into EasyRPG:master Apr 17, 2021
@ghost ghost deleted the scene-battle2k3-changes branch April 17, 2021 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants