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

ChipSet and Battle positioning issues #1169

Closed
zell180 opened this Issue May 17, 2017 · 23 comments

Comments

Projects
None yet
6 participants
@zell180

zell180 commented May 17, 2017

Name of the game: Personal Project

Player platform: Windows and Android

Attach files (as a .zip archive or link them)

TestEasyRPG.zip

Describe the issue in detail and how to reproduce it:

Hi, our main coder prepared a mini-demo of two problems we are experiencing in Easy RPG only. The issues are:

  1. ChipSet: in the map there is a column with a hole inside. The column appears withuot holes in RPG Maker. The hero of the mini-demo will show you the bugged column (which is part of a ChipSet) while you play it;

  2. Battle positioning: in our game we implemented a FaceSet Animation System. We don't use static FaceSets but we do use Battle animations to see them moving (mouth, eyes, etc). In the mini-demo the hero will talk for the first time and the animated FaceSet will appear in the right position (in its box of our text message picture). Then if you walk to the right, after the column, the game will take control of the hero, change the camera displacement and then move the hero upwards. He will talk again and this time the animated Battle of the FaceSet will not appear in the box. THIS DOES NOT HAPPEN IN RPG MAKER. To show the animated Battle of the FaceSet, throughout our game we use Event 0001 of every map as its target. Of course this event need to be repositioned every time we open a new message box, expecially in big maps, in order to see the animation played inside the box. This repositioning is performed by a parallel process (Common Event number 894, you find it in the test case). In RPG Maker this algorith always work, in any map and in any condition. In Easy RPG the result of the calculation is wrong because the Battle appears in different, wrong, positions (not in the message box). We are experiencing this problem in big maps, the ones which left the DEBUG message "Image size out of the boundary" in the Easy RPG log.

Please check the mini-demo and let us know where the differences with RPG Maker are and if you can fix them.
Thank you!

@scurest

This comment has been minimized.

Show comment
Hide comment
@scurest

scurest May 18, 2017

Contributor

Really nice testcase :)

1 is caused because the tile for the event is the very first one in the tileset (tile_id is 0) and we assume that the first tile is always fully transparent. A simple patch to Sprite_Character should fix this.

Contributor

scurest commented May 18, 2017

Really nice testcase :)

1 is caused because the tile for the event is the very first one in the tileset (tile_id is 0) and we assume that the first tile is always fully transparent. A simple patch to Sprite_Character should fix this.

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry May 18, 2017

Member

I second this. Great testcase. Usually we don't get such detailed reports :)

And battle animations for FaceSets is smart.

Member

Ghabry commented May 18, 2017

I second this. Great testcase. Usually we don't get such detailed reports :)

And battle animations for FaceSets is smart.

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry May 18, 2017

Member

After the panning EasyRPG populated the variables that receive ScreenXY and HeroXY incorrectly.

RPG_RT:

SX184
SY176
HX71
HY12

EasyRPG:

Screen X: 152
Screen Y: 180
Hero X: 69
Hero Y: 14
Member

Ghabry commented May 18, 2017

After the panning EasyRPG populated the variables that receive ScreenXY and HeroXY incorrectly.

RPG_RT:

SX184
SY176
HX71
HY12

EasyRPG:

Screen X: 152
Screen Y: 180
Hero X: 69
Hero Y: 14
@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry May 18, 2017

Member

ACtually the pan is wrong, too:
screenshot_20170518_110339

Member

Ghabry commented May 18, 2017

ACtually the pan is wrong, too:
screenshot_20170518_110339

@Tux80

This comment has been minimized.

Show comment
Hide comment
@Tux80

Tux80 May 18, 2017

Hi guys, I'm Tux, main coder of this game. I write because there is something strange... On EasyRPG (Android) I don't see the camera/pan like Ghabry (his image on the right). I see it exactly like the image on the left (which I guess is RPG_RT), but without the animated FaceSet ...

Tux80 commented May 18, 2017

Hi guys, I'm Tux, main coder of this game. I write because there is something strange... On EasyRPG (Android) I don't see the camera/pan like Ghabry (his image on the right). I see it exactly like the image on the left (which I guess is RPG_RT), but without the animated FaceSet ...

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry May 18, 2017

Member

hu, that's even more strange. o.O

Member

Ghabry commented May 18, 2017

hu, that's even more strange. o.O

@Tux80

This comment has been minimized.

Show comment
Hide comment
@Tux80

Tux80 May 18, 2017

This is how I see it on my Galaxy S7 (EasyRPG - Android).

18553963_10154979646914597_702432780_o

Tux80 commented May 18, 2017

This is how I see it on my Galaxy S7 (EasyRPG - Android).

18553963_10154979646914597_702432780_o

@zell180

This comment has been minimized.

Show comment
Hide comment
@zell180

zell180 May 18, 2017

This is my screen with apk of issue 1165. Galaxy S8
easyrpg player_2017-05-18-20-40-49

zell180 commented May 18, 2017

This is my screen with apk of issue 1165. Galaxy S8
easyrpg player_2017-05-18-20-40-49

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry May 18, 2017

Member

You are right. I get the same result as you now. No idea what went wrong when I made the screenshot.
But the Pan is still wrong in EasyRPG (and I guess this is the problem because you use an event to position the battle animation/faceset so when the screen is misaligned this will just not work)

Here is a screenshot from the editor which shows the tile-grid:
captura

A Pan command can only move in "number of tiles".
Here is the screen after the pan command:

captura 2png

As you can see the EasyRPG screen is misaligned and not on the tile grid :(

Member

Ghabry commented May 18, 2017

You are right. I get the same result as you now. No idea what went wrong when I made the screenshot.
But the Pan is still wrong in EasyRPG (and I guess this is the problem because you use an event to position the battle animation/faceset so when the screen is misaligned this will just not work)

Here is a screenshot from the editor which shows the tile-grid:
captura

A Pan command can only move in "number of tiles".
Here is the screen after the pan command:

captura 2png

As you can see the EasyRPG screen is misaligned and not on the tile grid :(

@Tux80

This comment has been minimized.

Show comment
Hide comment
@Tux80

Tux80 May 19, 2017

I see...so I guess that the solution for this won't be easy :(

As for the ChipSet problem, instead, you are already fixing it, did I get it right?

Tux80 commented May 19, 2017

I see...so I guess that the solution for this won't be easy :(

As for the ChipSet problem, instead, you are already fixing it, did I get it right?

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry May 19, 2017

Member

ChipSet problem: We don't give estimates when we fix some issues but that one appears simple to fix.

Member

Ghabry commented May 19, 2017

ChipSet problem: We don't give estimates when we fix some issues but that one appears simple to fix.

@Tux80

This comment has been minimized.

Show comment
Hide comment
@Tux80

Tux80 May 19, 2017

Thank you!
Keep us posted about both issues :)

Tux80 commented May 19, 2017

Thank you!
Keep us posted about both issues :)

@scurest

This comment has been minimized.

Show comment
Hide comment
@scurest

scurest May 20, 2017

Contributor

Ghabry, there's a diagonal row of events on the steps that trigger the hero to walk down into the arena and where you end up depends on which of these events you touch. You just touched a different one to get to different places. This is actually important. If you touch the top-most one, the face doesn't get displayed incorrectly! This is a hint, because it's also the only one where the pan never goes below the bottom edge of the map.

Here's what's wrong. The way we update the scroll is (looking only at the x direction)

if (last_pan_x != current_pan_x) {
     ScrollRight(current_pan_x - last_pan_x);
     last_pan_x = current_pan_x;
}

Now suppose we stand near the right edge of the map and do a pan to the right, wait for the pan to finish, then reset the pan, and wait for the pan to finish. In RPG_RT, the screen doesn't move. In Player, this will do a bunch of ScrollRight(...)s followed by a bunch of ScrollLeft(...)s which would normally cancel out. But because we're at the right edge of the map, the ScrollRights don't have any effect, because we don't move over the right edge, so there's nothing for the ScrollLefts to cancel out, so the net effect is to move the screen position to the left. So when the pan goes off screen, and then comes back on, we mess up the screen position.

I'm not quite sure why the tiles wind up misaligned though...

To compare, Game_Player::Center puts the screen at the right place but doesn't handle scrolling the BG or the hero walking. Using it does get the face to show up in the right place though.

I'll try writing up something to fix this later.

Contributor

scurest commented May 20, 2017

Ghabry, there's a diagonal row of events on the steps that trigger the hero to walk down into the arena and where you end up depends on which of these events you touch. You just touched a different one to get to different places. This is actually important. If you touch the top-most one, the face doesn't get displayed incorrectly! This is a hint, because it's also the only one where the pan never goes below the bottom edge of the map.

Here's what's wrong. The way we update the scroll is (looking only at the x direction)

if (last_pan_x != current_pan_x) {
     ScrollRight(current_pan_x - last_pan_x);
     last_pan_x = current_pan_x;
}

Now suppose we stand near the right edge of the map and do a pan to the right, wait for the pan to finish, then reset the pan, and wait for the pan to finish. In RPG_RT, the screen doesn't move. In Player, this will do a bunch of ScrollRight(...)s followed by a bunch of ScrollLeft(...)s which would normally cancel out. But because we're at the right edge of the map, the ScrollRights don't have any effect, because we don't move over the right edge, so there's nothing for the ScrollLefts to cancel out, so the net effect is to move the screen position to the left. So when the pan goes off screen, and then comes back on, we mess up the screen position.

I'm not quite sure why the tiles wind up misaligned though...

To compare, Game_Player::Center puts the screen at the right place but doesn't handle scrolling the BG or the hero walking. Using it does get the face to show up in the right place though.

I'll try writing up something to fix this later.

scurest added a commit to scurest/Player that referenced this issue May 20, 2017

Always draw event sprites, even for tile_id = 0
Sprite_Character was mostly conditioned on tile_id == 0 to
distinguish CharSet sprites vs. tiles. Now, it's mostly conditioned
on character_name.empty().

Fixes the first part of #1169.
@scurest

This comment has been minimized.

Show comment
Hide comment
@scurest

scurest May 21, 2017

Contributor

Anyone know what the StartScroll, UpdateScroll, &c. stuff in Game_Map is supposed to be for? grep StartScroll src/*.cpp returns only the definition. I checked out the commit from when they were last touched (6 years ago) and they didn't seem to be used then either...

Contributor

scurest commented May 21, 2017

Anyone know what the StartScroll, UpdateScroll, &c. stuff in Game_Map is supposed to be for? grep StartScroll src/*.cpp returns only the definition. I checked out the commit from when they were last touched (6 years ago) and they didn't seem to be used then either...

scurest added a commit to scurest/Player that referenced this issue May 21, 2017

Always draw event sprites, even for tile_id = 0
Sprite_Character was mostly conditioned on tile_id == 0 to
distinguish CharSet sprites vs. tiles. Now, it's mostly conditioned
on character_name.empty().

Fixes the first part of #1169.

scurest added a commit to scurest/Player that referenced this issue May 21, 2017

Revamp handling of scrolling on map screen
Instead of trying to calculate how much to scroll the screen,
calculate where it should be centered and derive the amount
moved from that.

Most of the scrolling code, plus the caches for old values in
Game_Player, were thus rendered unnecessary.

Now correctly handles pans that are out of the map, fixing the
second part of #1169.
@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry May 21, 2017

Member

Anyone know what the StartScroll, UpdateScroll, &c. stuff in Game_Map is supposed to be for?

Nope, most of the map code is a mistery to me. I think glynnc wrote most of it :D

Member

Ghabry commented May 21, 2017

Anyone know what the StartScroll, UpdateScroll, &c. stuff in Game_Map is supposed to be for?

Nope, most of the map code is a mistery to me. I think glynnc wrote most of it :D

@zell180

This comment has been minimized.

Show comment
Hide comment
@zell180

zell180 May 21, 2017

Can you attach to us a version of easyrpg including patches to test it? Apk and Windows. Thanks very much

zell180 commented May 21, 2017

Can you attach to us a version of easyrpg including patches to test it? Apk and Windows. Thanks very much

@scurest

This comment has been minimized.

Show comment
Hide comment
@scurest

scurest May 21, 2017

Contributor

Well, I deleted it all :)

I'll PR this branch after I do some more testing (taking bets for number of regression this time ;))

Contributor

scurest commented May 21, 2017

Well, I deleted it all :)

I'll PR this branch after I do some more testing (taking bets for number of regression this time ;))

scurest added a commit to scurest/Player that referenced this issue May 21, 2017

Revamp handling of scrolling on map screen
Instead of trying to calculate how much to scroll the screen,
calculate where it should be centered and derive the amount
moved from that.

Most of the scrolling code, plus the caches for old values in
Game_Player, were thus rendered unnecessary.

Now correctly handles pans that are out of the map, fixing the
second part of #1169.
@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry May 21, 2017

Member

When you open the PR I will make some tests on the TestGame looping Pan map because there were many rendering bugs (events missing) when you panned over the map border.

Member

Ghabry commented May 21, 2017

When you open the PR I will make some tests on the TestGame looping Pan map because there were many rendering bugs (events missing) when you panned over the map border.

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry May 21, 2017

Member

Another test candidate: The Wolfenhain boat minigame (misaligned by 1 tile)

Member

Ghabry commented May 21, 2017

Another test candidate: The Wolfenhain boat minigame (misaligned by 1 tile)

@scurest

This comment has been minimized.

Show comment
Hide comment
@scurest

scurest May 21, 2017

Contributor

That's would be awesome, thanks :)

The boat minigame appears fixed (funny glitch btw).

Contributor

scurest commented May 21, 2017

That's would be awesome, thanks :)

The boat minigame appears fixed (funny glitch btw).

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry May 21, 2017

Member

Yepp can also confirm that both bugs are fixed. But the Pan test-map has a new bug (I think, not checked against RPG_RT):
One event invokes "Fix Map" and when you "Unfix Map" with a different event the map jumps directly in the middle. I think in RPG_RT the map stays at the fixed position and "unfixes" when the player scrolls the map.

Member

Ghabry commented May 21, 2017

Yepp can also confirm that both bugs are fixed. But the Pan test-map has a new bug (I think, not checked against RPG_RT):
One event invokes "Fix Map" and when you "Unfix Map" with a different event the map jumps directly in the middle. I think in RPG_RT the map stays at the fixed position and "unfixes" when the player scrolls the map.

@scurest

This comment has been minimized.

Show comment
Hide comment
@scurest

scurest May 23, 2017

Contributor

So always centering the screen doesn't work with the "Fix Map" stuff, so I'm giving that up :( I'll PR the fix for the first part first, and do the second one in a different PR, because the second one will probably be harder to review.

Contributor

scurest commented May 23, 2017

So always centering the screen doesn't work with the "Fix Map" stuff, so I'm giving that up :( I'll PR the fix for the first part first, and do the second one in a different PR, because the second one will probably be harder to review.

@Tux80

This comment has been minimized.

Show comment
Hide comment
@Tux80

Tux80 May 25, 2017

Hello guys, I have tested the ChipSet problem using the last successfull nightly build for the Android Player and now the column appears to be in good shape :)
Well done!

As for the second bug (Battle/FaceSet animation not positioned correctly) I understood that the problem is not easy to fix and has a high regression bug risk, so you are taking your time to fix/test.

We well be following this issue, keep us posted as soon as you have some news, thanks!

Tux80 commented May 25, 2017

Hello guys, I have tested the ChipSet problem using the last successfull nightly build for the Android Player and now the column appears to be in good shape :)
Well done!

As for the second bug (Battle/FaceSet animation not positioned correctly) I understood that the problem is not easy to fix and has a high regression bug risk, so you are taking your time to fix/test.

We well be following this issue, keep us posted as soon as you have some news, thanks!

scurest added a commit to scurest/Player that referenced this issue May 30, 2017

Track how much of the pan has actually occurred
This fixes the problem where the pan moves too far when it goes
off the screen and come back on. Fixes #1169.

@Ghabry Ghabry closed this in #1197 Jun 10, 2017

Ghabry pushed a commit to libretro/easyrpg-libretro that referenced this issue May 22, 2018

Always draw event sprites, even for tile_id = 0
Sprite_Character was mostly conditioned on tile_id == 0 to
distinguish CharSet sprites vs. tiles. Now, it's mostly conditioned
on character_name.empty().

Fixes the first part of #1169.

Ghabry pushed a commit to libretro/easyrpg-libretro that referenced this issue May 22, 2018

Track how much of the pan has actually occurred
This fixes the problem where the pan moves too far when it goes
off the screen and come back on. Fixes #1169.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment