Skip to content

Conversation

wvpm
Copy link
Contributor

@wvpm wvpm commented Jul 15, 2025

#496 uses the index from HasIndex to directly index arrays. This is great for performance but requires 0-based indexing.
Provinces use 1-based indexing for their 'external id' (data-loading, UI & console).

To remedy this, provinces now use 0-based indices for HasIndex and clearly marked 1-based indices for pathfinding, console, game actions and such.

@wvpm wvpm added this to the 0.9.0 milestone Jul 15, 2025
@wvpm wvpm added bug Something isn't working topic:core labels Jul 15, 2025
@wvpm wvpm force-pushed the distinguish_between_0_1_based_indexing branch from 8a21163 to d5f3e89 Compare July 15, 2025 13:20
@wvpm wvpm force-pushed the distinguish_between_0_1_based_indexing branch from d5f3e89 to 096ee5b Compare July 15, 2025 13:22
@wvpm wvpm enabled auto-merge July 15, 2025 13:23
@Spartan322
Copy link
Member

I feel like we should have instead do a separate name for index and position, index is 0-based, position is 1-based.

@wvpm
Copy link
Contributor Author

wvpm commented Jul 19, 2025

I feel like we should have instead do a separate name for index and position, index is 0-based, position is 1-based.

As mentioned in the discord, renaming the 1-based value to province id or something similar seems fine.
Currently the 1-based value is still heavily used for array indexing.

It should be refactored to use the 0-based index. That requires too much effort for a bug fix.
Let's fix the bug first and refactor it later.

@wvpm wvpm merged commit 2d52540 into master Jul 25, 2025
16 checks passed
@wvpm wvpm deleted the distinguish_between_0_1_based_indexing branch July 25, 2025 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working topic:core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants