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

Enumerate the map tiles in row order #3641

Merged
merged 2 commits into from
Feb 20, 2024

Conversation

past-due
Copy link
Member

@past-due past-due commented Feb 16, 2024

Most cases where we loop over the map tiles do so in memory order, but a few reversed the nested loops (resulting in enumerating in column order, which is not the order in memory). Fix this.

@past-due past-due force-pushed the 2024_02_maptile_enum_tweaks_1 branch from 9b6fea5 to fb7604c Compare February 16, 2024 21:50
src/map.cpp Fixed Show fixed Hide fixed
@ManManson
Copy link
Member

Can you please explain what is the motivation behind the change?

@Cyp
Copy link
Member

Cyp commented Feb 17, 2024

Can you please explain what is the motivation behind the change?

It's conventional to go through arrays in the order that they're stored in memory, because it's often better for the CPU memory cache and therefore faster. Random relevant QA here: https://stackoverflow.com/questions/11148575/why-is-it-worse-to-initialize-a-two-dimensional-array-like-this

@ManManson
Copy link
Member

Sure. The reason why I've asked this is because it wasn't immediately visible to me what was the original layout of arrays affected by the PR (Without carefully reading into the diff and surrounding code; e.g., theoretically, there are cases where optimal physical data orientation is column-wide instead of row-wide).

Just a bit more context either here in the PR description or in the code comments section would be great.

@past-due past-due force-pushed the 2024_02_maptile_enum_tweaks_1 branch from fb7604c to fd6e96d Compare February 20, 2024 17:54
@past-due past-due merged commit 8477d83 into Warzone2100:master Feb 20, 2024
38 checks passed
@past-due past-due deleted the 2024_02_maptile_enum_tweaks_1 branch February 20, 2024 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants