Fixes for Rainbow Treasure OP #428

Merged
merged 5 commits into from Feb 26, 2015

Projects

None yet

4 participants

@scurest
Contributor
scurest commented Feb 26, 2015

Fixes the errors I noticed in the Rainbow Treasure OP.

Character placement off f1fe079

tear

Some events are drawn off by one pixel. Caused by the formula in Game_Character::GetScreenX() and Y(). Changed to take the difference in pixels, not real coordinates, which I think is correct. Can someone check?

Pan speed too slow be05554

pan

The pan drags behind the characters during the OP. Changed just by increasing the pan speed, but also changed to use SCREEN_TILE_WIDTH / 128 instead of 2, like for characters, I think it goes here too: @Ghabry, you made that change for character speed, what do you think?

Pan jumps after map teleport 8efd1a5

copper

Caused by the pan in Game_Player being reset too early. I think this also fixes a bug where the pan is lost when teleporting within the same map (?).

Parallax BGs don't scroll backwards 28cedf8

manganese

Manganese's OP uses a parallax background to make it look like she's walking, but it goes the wrong way! Just a missing sign in UpdateParallax but I also used SCREEN_TILE_WIDTH / 128 here and sped it up.

Jumping in place 716623d

jump

Discussed here.


There's also #356 and one more I just noticed—the old man in front of the chest's sweat drop is drawn behind him when it should be drawn in front—that I didn't fix.

@Ghabry Ghabry commented on the diff Feb 26, 2015
src/game_character.cpp
}
int Game_Character::GetScreenY() const {
- int y = (real_y - Game_Map::GetDisplayY() + 3) / (SCREEN_TILE_WIDTH / TILE_SIZE) + TILE_SIZE;
+ int y = real_y / (SCREEN_TILE_WIDTH / TILE_SIZE) - Game_Map::GetDisplayY() / (SCREEN_TILE_WIDTH / TILE_SIZE) + TILE_SIZE;
@Ghabry
Ghabry Feb 26, 2015 Member

This can be simplified to

(real_y - Game_Map::GetDisplayY()) / (SCREEN_TILE_WIDTH / TILE_SIZE) + TILE_SIZE;

(same for x)

@Zegeri
Zegeri Feb 26, 2015 Member

a / (b / c) = a * c / b. So that can be rewritten as:

((real_y - Game_Map::GetDisplayY()) / SCREEN_TILE_WIDTH + 1) * TILE_SIZE;
@Ghabry
Ghabry Feb 26, 2015 Member

Finally a practical use for school math.

@scurest
scurest Feb 26, 2015 Contributor

Unfortunately integer division doesn't distribute over addition, eg. take SCREEN_TILE_WIDTH/TILE_SIZE=16, GetDisplayY()=1, and real_y=16. Then 16/16 - 1/16 = 1, but (16-1)/16 = 0.

@fdelapena
Member

Thanks scurest, about the swear drop issue is possibly related with #333.

@Ghabry
Member
Ghabry commented Feb 26, 2015

+1 for this really detailed pull request.
Are you aware of any further open issue that are fixed by this?

@Ghabry Ghabry merged commit d251d40 into EasyRPG:master Feb 26, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
default Merged build finished.
Details
@scurest
Contributor
scurest commented Feb 26, 2015

I'm pretty sure no open issues are closed by this one, haha.

By the way... (I've never used Github before) would it have been better to open individual issues for all these and put details there?

@fdelapena
Member

I think there are a good collection of visible bugs not reported in the issue tracker which can be fixed "on the fly" without needing to open a separated bug before and it saves development time. They can be reported before, but I think it is not necessary in the current development stage. A lot of code contributed didn't have an issue previously opened and it can be discussed in pull requests too :) .

@Ghabry
Member
Ghabry commented Feb 27, 2015

I usually only open issues for problems where I'm certain that there won't be a fix from my side soon.
When you fix some unreported problems just do it like in this PR

@scurest scurest deleted the scurest:RainbowTreasureOP branch Mar 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment