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

Scrolling down/right out of map on a 255 high flat map bounces viewport back to edge of map #6583

Closed
DorpsGek opened this issue Jul 8, 2017 · 7 comments

Comments

@DorpsGek
Copy link

@DorpsGek DorpsGek commented Jul 8, 2017

james1101 opened the ticket and wrote:

When you scroll down right (SE) on a 255-high flat map, instead of stopping at the edge, the game lets me continue about another few hundred pixels before bouncing your viewport back to the edge of map.

How to Reproduce:

  1. Download the attached save and load it into OpenTTD.
  2. Scroll down right out of the edge of the map.
    After a couple hundred pixels out of bounds, the viewport bounces back to the edge of the map.

Possible Reason: The tile height isn't taken into account when checking for viewport out of bounds SE and SW. But after a couple hundred pixels, the tile to scroll to is calculated once the game realizes the viewport is out of bounds. The viewport scrolls back to that calculated tile, inside the map, taking the height into account. The threshold for number of pixels out before bounce back varies with the height of the tiles at the SW and SE edges.

Also applies for the following directions: Down left (SW), and straight down (S). But not up left (NW), up right (NE), or straight up (N).

Using Win 7 Ultimate 32 bit.

Attachments

Reported version: 1.7.1
Operating system: Windows


This issue was imported from FlySpray: https://bugs.openttd.org/task/6583
@DorpsGek

This comment has been minimized.

Copy link
Author

@DorpsGek DorpsGek commented Jul 10, 2017

adf88 wrote:

More height levels overcomplicated some things a bit.
I removed faulty piece of it - ClampXYToMap is gone. We already had similar functionality, I just had to extract it out of TranslateXYToTileCoord. And so I did, new InverseRemapCoords2 function (inverse of RemapCoords2) works like a charm.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6583#comment14465
@DorpsGek

This comment has been minimized.

Copy link
Author

@DorpsGek DorpsGek commented Jul 10, 2017

adf88 wrote:

Just one correction - foundations shouldn't be taken into account so I added a switch to InverseRemapCoords2 that allows to disable them.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6583#comment14466
@DorpsGek

This comment has been minimized.

Copy link
Author

@DorpsGek DorpsGek commented Jul 15, 2017

adf88 wrote:

I further investigated the code and this is a deep problem in general. "More height levels" is reinventing the wheel (duplicated functionality) and fixes imaginary problems (like the terraforming "issue"). Currently I'm yak-shaving this and removing/unifying stuff. Some functions return inconsistent values. TileHeight and TileHeightOutsideMap return different values for the same tile (when at southern border). TranslateXYToTileCoord has totally different model for tiles outside map then *OutsideMap function family. Tile slopes are also not consistent, based on method used to read it.

I would like to change the code in a way that tile heights at map edges always matter and TileHeight is always a "valid" value. Make sure that all functions return consistent values when called on the same tiles. Make sure that functions return consistent values when called on neighbor tiles (near map edges too). Unify "tile outside map" model. Deduplicate and removed unneeded "more height levels" stuff.

In the end, I change my mind about foundations - they don't harm and may stay just for convenience.


This comment was imported from FlySpray: https://bugs.openttd.org/task/6583#comment14469
@DorpsGek

This comment has been minimized.

Copy link
Author

@DorpsGek DorpsGek commented Jul 22, 2017

adf88 wrote:

How I've said, I did.

Here's entire patch queue.

// EDIT: SORRY, BROKEN UPLOAD, JUST GRAB V2 INSTEAD

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6583#comment14472
@andythenorth

This comment has been minimized.

Copy link
Contributor

@andythenorth andythenorth commented Jan 12, 2019

Seems like this has patches for both?

  • viewport scroll fix
  • refactoring of tile heights
@andythenorth

This comment has been minimized.

Copy link
Contributor

@andythenorth andythenorth commented Jan 12, 2019

Thanks for this. There's been no activity on this for some time, and as it stands, it doesn't look likely that it will go any further. Since OpenTTD moved to GitHub, we use pull requests rather than patches, as they are a much more productive workflow.

I'm planning to close this soon (in 7 days), as we try to keep the issue count low for OpenTTD, it helps us focus on things that are important and fun.

If you would like to continue with this patch, the best way would be to move the patch to your own GitHub fork, update it for the current OpenTTD master, and then create a pull request. For more information, please see our CONTRIBUTING.md.

We are also happy to discuss directly on the issue, or in #openttd irc, including help to get this into a pull request. Thanks for your contribution!

Eddi-z added a commit to Eddi-z/OpenTTD that referenced this issue Jan 13, 2019
Eddi-z added a commit to Eddi-z/OpenTTD that referenced this issue Jan 13, 2019
…oordinates to underlying tile coordinates (Patch by adf88, OpenTTD#6583)
Eddi-z added a commit to Eddi-z/OpenTTD that referenced this issue Jan 13, 2019
Eddi-z added a commit to Eddi-z/OpenTTD that referenced this issue Jan 13, 2019
Eddi-z added a commit to Eddi-z/OpenTTD that referenced this issue Jan 13, 2019
…wn as fast as possible" tile height model (Patch by adf88, OpenTTD#6583)
@andythenorth

This comment has been minimized.

Copy link
Contributor

@andythenorth andythenorth commented Jan 24, 2019

Patches moved to #7061

nielsmh added a commit that referenced this issue Jan 24, 2019
nielsmh added a commit that referenced this issue Jan 24, 2019
…oordinates to underlying tile coordinates (Patch by adf88, #6583)
nielsmh added a commit that referenced this issue Jan 24, 2019
nielsmh added a commit that referenced this issue Jan 24, 2019
…wn as fast as possible" tile height model (Patch by adf88, #6583)
@nielsmh nielsmh closed this Jan 24, 2019
PeterN added a commit to PeterN/OpenTTD that referenced this issue Feb 3, 2019
…k ViewportAddLandscape so it no more relies on "go down as fast as possible" tile height model (Patch by adf88, OpenTTD#6583)"

This reverts commit 479f13f.
PeterN added a commit that referenced this issue Feb 3, 2019
…andscape so it no more relies on "go down as fast as possible" tile height model (Patch by adf88, #6583)"

This reverts commit 479f13f.
nielsmh added a commit to nielsmh/OpenTTD that referenced this issue Mar 11, 2019
nielsmh added a commit to nielsmh/OpenTTD that referenced this issue Mar 11, 2019
…oordinates to underlying tile coordinates (Patch by adf88, OpenTTD#6583)
nielsmh added a commit to nielsmh/OpenTTD that referenced this issue Mar 11, 2019
nielsmh added a commit to nielsmh/OpenTTD that referenced this issue Mar 11, 2019
nielsmh added a commit to nielsmh/OpenTTD that referenced this issue Mar 11, 2019
…wn as fast as possible" tile height model (Patch by adf88, OpenTTD#6583)
nielsmh added a commit to nielsmh/OpenTTD that referenced this issue Mar 11, 2019
…k ViewportAddLandscape so it no more relies on "go down as fast as possible" tile height model (Patch by adf88, OpenTTD#6583)"

This reverts commit 479f13f.
ZornsLemma added a commit to ZornsLemma/OpenTTD that referenced this issue Jul 12, 2019
I think the code is fine; noting that the second argument to
ExecuteTerraforming() is false, the code is really very similar to that
created by commit 05da5a1 "Codechange:
Simplify marking tiles dirty when terraforming (Patch by adf88, OpenTTD#6583)".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.