-
Notifications
You must be signed in to change notification settings - Fork 494
exclude antinuke from area select settings opton #6286
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
exclude antinuke from area select settings opton #6286
Conversation
|
It's a bit unsettling to me, how many places have to be changed to include such a feature. Maybe there is room for some refactoring? |
|
more toggles 🎉 |
| udid = spGetUnitDefID(uid) | ||
| if buildingFilter[udid] == false then | ||
| if includeBuilders or not builderFilter[udid] then | ||
| if (includeBuilders or not builderFilter[udid]) and (includeAntinuke or not antinukeFilter[udid]) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't even merged and has already set a bad precedent (see #6309), but this would've been fine in isolation so let's let it be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe instead of checking if this and if this and if this, make a table of all exclusions and have a single key lookup?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just commented on my PR but will put here too, happy to refactor this if preferred before merging future tech debt 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! Steal whatever code you need I'll close this one. Marking as draft for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have included these in my PR now, thanks @SethDGamre ❤️
|
Included into #6309 |
Attributing SethDGamre beyond-all-reason#6286 Co-authored-by: SethDGamre <[email protected]>
defaults to excluding antinuke, because I keep seeing the same thread over and over again with reputable players like Chronopolize and Blodir agreeing that you wouldn't want to area-select antinuke with your combat units. See the most recent incarnation: https://discord.com/channels/549281623154229250/1441111177605156986
This adds a settings toggle to do that.
2025-11-22.07-50-44.mp4