The Wayback Machine - https://web.archive.org/web/20220604104021/https://github.com/OpenRCT2/OpenRCT2/issues/16840
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

Support rectangular heightmaps #16840

Open
Broxzier opened this issue Mar 21, 2022 · 5 comments
Open

Support rectangular heightmaps #16840

Broxzier opened this issue Mar 21, 2022 · 5 comments
Labels
enhancement good first issue

Comments

@Broxzier
Copy link
Member

@Broxzier Broxzier commented Mar 21, 2022

With support for rectangular maps added to the game, the requirement for heightmaps to be square is no longer necessary.

auto image = Imaging::ReadFromFile(path, format);
if (image.Width != image.Height)
{
context_show_error(STR_HEIGHT_MAP_ERROR, STR_ERROR_WIDTH_AND_HEIGHT_DO_NOT_MATCH, {});
return false;
}

The code below the above check relies on the assumption that the width and height match. This needs to be refactored slightly. Some changes to the related functions might be necessary, though most already threat width and height separately.

openrct2_assert(_heightMapData.width == _heightMapData.height, "Invalid height map size");

@Broxzier Broxzier added enhancement good first issue labels Mar 21, 2022
@Broxzier Broxzier changed the title Support non-square heightmaps Support rectangular heightmaps Mar 21, 2022
@ArLough
Copy link

@ArLough ArLough commented Apr 2, 2022

Hi, I'm new to contributing and I'd like to take this issue. Is there anything else I should do to take the issue?

@Broxzier
Copy link
Member Author

@Broxzier Broxzier commented Apr 3, 2022

@ArLough nope, that's all you have to do. For questions you can ask either here or on Discord. Good luck!

@ArLough
Copy link

@ArLough ArLough commented Apr 13, 2022

Hi @Broxzier, I was wondering if you would like me to remove the error checking entirely or substitute it with a different error check that takes rectangular maps into consideration? If the latter, what would you like the error checking to look like?

@Broxzier
Copy link
Member Author

@Broxzier Broxzier commented Apr 14, 2022

Hey @ArLough, the checks are not necessary anymore, so they can be removed entirely.

Edit: Actually, there is a check for map size, but this only checks the width. For rectangular map support, this also needs to check the height, and also clamp them correctly.

auto size = image.Width;
if (image.Width > MAXIMUM_MAP_SIZE_PRACTICAL)
{
context_show_error(STR_HEIGHT_MAP_ERROR, STR_ERROR_HEIHGT_MAP_TOO_BIG, {});
size = std::min<uint32_t>(image.Height, MAXIMUM_MAP_SIZE_PRACTICAL);
}

norisa118 added a commit to norisa118/OpenRCT2 that referenced this issue Apr 18, 2022
Substitute checks that assumed height = width with new checks
distinguishing width from height
@tupaschoal
Copy link
Member

@tupaschoal tupaschoal commented May 4, 2022

For anyone reading, this is up for grabs again. It might be worth looking into the comments of the PR linked above for insight

Broxzier added a commit to Broxzier/OpenRCT2 that referenced this issue May 11, 2022
Broxzier added a commit to Broxzier/OpenRCT2 that referenced this issue May 11, 2022
Broxzier added a commit to Broxzier/OpenRCT2 that referenced this issue May 11, 2022
Broxzier added a commit to Broxzier/OpenRCT2 that referenced this issue May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good first issue
3 participants