Skip to content

Conversation

@avezey-ci
Copy link
Contributor

Summary

  • allow quick start range override to be disabled by keeping the min at 0
  • parse override range safely and fall back to the default when unset

Testing

  • docker compose -f tools/headless_testing/docker-compose.yml build
  • docker compose -f tools/headless_testing/docker-compose.yml run --rm bar

@github-actions
Copy link
Contributor

github-actions bot commented Nov 19, 2025

Test Results

14 tests  ±0   7 ✅ ±0   9s ⏱️ ±0s
 1 suites ±0   7 💤 ±0 
 1 files   ±0   0 ❌ ±0 

Results for commit 034f221. ± Comparison against base commit ea60385.

♻️ This comment has been updated with latest results.

@SethDGamre
Copy link
Collaborator

I can see how my mod option here was kind of silly for not letting you set it back to zero after changing it.

It is really important that there is enough space for a base to be built around the commander. I want you to do modOptionRangeValue > 0 and math.max(modOptionRangeValue, 200) or DefaultRange

Declare CONSTANTS and use the real variable names as necessary.

At ranges below 200, the base layouts are just too tight to spawn anything with the other factors involved. Even 200 might be too tight, you should test that. Especially look at AI generated bases.

@sprunk
Copy link
Collaborator

sprunk commented Nov 20, 2025

Using math.max means you break WYSIWYG and values between 1 and 199 are fake though.

@SethDGamre
Copy link
Collaborator

SethDGamre commented Nov 20, 2025

Good point, update the description.
That's tough s***. Users do stupid stuff with modoptions all the time and whatever configurations they come up with has to be safety guarded against ignorant decisions that don't just affect the configurer, but the entire lobby of people trying the feature.

@SethDGamre
Copy link
Collaborator

Maybe a Boolean modoption to show and enable the override and leave the min in the modoption?

@avezey-ci
Copy link
Contributor Author

avezey-ci commented Nov 20, 2025

I see two options:

  1. Clamp unsafe values: keep the numeric modoption but treat override > 0 as max(override, 200) and say so in the description.

Rationale: the gadget uses INSTANT_BUILD_RANGE = override > 0 and override or 600 while base generation is tuned for ~500–600 (for example BASE_GENERATION_RANGE = 500, COMMANDER_NO_GO_DISTANCE = 100, BUILD_SPACING = 64), so ranges under ~200 cannot lay out a viable base, especially for AI, so coercing <200 protects lobbies.

  1. Boolean gate: add a boolean “allow override” plus the range field, but even then we would still need to guard low values to avoid failed spawns, so it does not really fix the safety issue.

Given the code paths, I think (1) is the safer default despite the slight WYSIWYG hit for 1–199; we can call out the clamp in the modoption text and note that <200 will act as 200. If that direction sounds good, I will clamp and update the description, then sanity-check AI bases at 200–250.

@sprunk sprunk requested a review from WatchTheFort November 20, 2025 17:41
@SethDGamre
Copy link
Collaborator

SethDGamre commented Nov 20, 2025

I would want:

  1. modOption.enableQuickstartOverrides, Boolean, default false
  2. modoption.overrideQuickStartRange, number, default 600, min 200, max 2000
  3. modoption.overrideQuickStartBudget, number, default 1200, min 100, no max

If enableQuickStartOverrides == true then use the overrides. Else, use the string-matched table entries for budget. And default quick start build range.

All of these options hidden, I don't want the average player to discover and use them. It's for debugging or modding purposes.

@avezey-ci
Copy link
Contributor Author

@SethDGamre ok, I implemented hidden quick start overrides per your request: added enable_quickstart_overrides (default false) as the gate, plus override_quick_start_range (default 600, min 200, max 2000) and override_quick_start_budget (default 1200, min 100). When the flag is off I use the normal quick_start_amount table and default 600 range; when on we use the override values. All options are hidden and the headless test suite passes.

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