Skip to content

Unit: iterate SResourcePack by index instead of std::views::enumerate - #3029

Closed
tomjn wants to merge 1 commit into
beyond-all-reason:masterfrom
tomjn:fix/unit-portable-enumerate
Closed

Unit: iterate SResourcePack by index instead of std::views::enumerate#3029
tomjn wants to merge 1 commit into
beyond-all-reason:masterfrom
tomjn:fix/unit-portable-enumerate

Conversation

@tomjn

@tomjn tomjn commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

What

SplitResourcePackIntoPositiveNegative in Unit.cpp iterated a SResourcePack with std::views::enumerate:

for (auto [resourceID, value] : std::views::enumerate (pack)) { ... }

This is the only std::ranges/std::views use in the file. It's replaced with a plain index loop:

for (int resourceID = 0; resourceID < SResourcePack::MAX_RESOURCES; ++resourceID) {
    const auto value = pack.res[resourceID];
    ...
}

Why

std::views::enumerate (C++23, P2164) is not available across all standard libraries we target — notably libc++ on Apple Clang. The index loop is portable, behavior-identical, and matches how the rest of the codebase already iterates SResourcePackrts/Sim/Misc/Resource.h uses for (int i = 0; i < MAX_RESOURCES; ++i) throughout. The enumerate call here was the odd one out.

MAX_RESOURCES is 2, so there is no readability cost.

Why this is cross-platform

No behavior change on any platform; master builds fine today, this just removes a libc++ portability blocker so no platform-specific fallback is needed downstream.

Provenance

Surfaced by the macOS port (#2991), which carried this same index loop as an __APPLE__-gated fallback (author: iamaperson000, credited via Co-authored-by). Applying it unconditionally means the macOS branch needs no gate here at all.

SplitResourcePackIntoPositiveNegative used std::views::enumerate, the
only std::ranges/std::views use in this file. The C++23 enumerate view
(P2164) is not available on all standard libraries we target (notably
libc++ on Apple Clang). Use a plain index loop over
SResourcePack::res[0..MAX_RESOURCES] instead.

This matches how the rest of the codebase already iterates SResourcePack
(see rts/Sim/Misc/Resource.h, which uses for (int i = 0; i <
MAX_RESOURCES; ++i) throughout), and removes a portability blocker
without changing behavior.

Surfaced by the macOS port (beyond-all-reason#2991), which carried this as an
__APPLE__-gated fallback; applying it unconditionally means no platform
gate is needed.

Co-authored-by: iamaperson000 <kanishkv99@gmail.com>
@sprunk

sprunk commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

I specifically used enumerate because ongoing resource work will decouple the underlying array from how many resources there actually are. Use d3dd051 instead.

@tomjn

tomjn commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Closing — @sprunk confirmed std::views::enumerate is intentional (the ongoing resource work decouples the backing array from the resource count), and the index loop here hardcodes exactly the direct pack.res[i] access that's being moved away from. The accepted path is the Recoil::enumerate polyfill in Cpp23Compat.hpp (commit d3dd051), so this standalone change isn't the right approach. Thanks for the context.

@tomjn tomjn closed this Jun 18, 2026
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.

2 participants