Skip to content

Add objective bag size to control objective repeats#22

Open
eiron wants to merge 2 commits intosilasary:keymasters_keepfrom
eiron:kk-objectivebagsizes
Open

Add objective bag size to control objective repeats#22
eiron wants to merge 2 commits intosilasary:keymasters_keepfrom
eiron:kk-objectivebagsizes

Conversation

@eiron
Copy link

@eiron eiron commented Feb 28, 2026

Introduce an objective bag size to allow objectives to repeat a configurable number of times across the keep.

Introduce an objective bag size to allow objectives to repeat a configurable number of times across the keep.
Copilot AI review requested due to automatic review settings February 28, 2026 01:10
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new configuration option to control how often identical objectives can repeat across a generated Keymaster’s Keep, threading that value from options → world generation → objective generator → per-game objective selection.

Changes:

  • Introduces ObjectiveSelectionBagSize option and plumbs it into world generation.
  • Reworks objective de-duplication state from a set to a Dict[str, int] to support “repeat up to N times”.
  • Updates objective generation for both standard games and Game Medley to honor the configured repeat cap.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
worlds/keymasters_keep/world.py Reads the new option during early generation and passes it into the objective generator.
worlds/keymasters_keep/options.py Adds the ObjectiveSelectionBagSize range option and registers it in option groups.
worlds/keymasters_keep/games/game_medley_game.py Tracks objective usage counts for medley objectives and applies the repeat cap.
worlds/keymasters_keep/game_objective_generator.py Adds an objective_bag_size parameter and forwards it into objective generation calls.
worlds/keymasters_keep/game.py Applies repeat-cap logic for non-medley objective generation using a count dictionary.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +142 to 146
objectives.append(prefixed_objective)
objectives_in_use[prefixed_objective] = objectives_in_use.get(prefixed_objective, 0) + 1

break

Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The passes_templates > 50 fallback appends prefixed_objective even when objective_bag_size has already been reached, so the configured repeat cap can be violated. Consider changing the fallback to respect the cap (e.g., fail with a clear message suggesting increasing objective_selection_bag_size / allowing repeats) rather than silently exceeding it.

Suggested change
objectives.append(prefixed_objective)
objectives_in_use[prefixed_objective] = objectives_in_use.get(prefixed_objective, 0) + 1
break
raise ValueError(
"Unable to select a new unique objective without exceeding the configured "
"'objective_bag_size'. Consider increasing 'objective_selection_bag_size' or "
"allowing objective repeats."
)

Copilot uses AI. Check for mistakes.
Comment on lines 897 to 909
game_objective_data: GameObjectiveGeneratorData = generator.generate_from_plan(
plan,
self.random,
self.game_selection_bag_size,
self.include_difficult_objectives,
self.excluded_games_difficult_objectives,
self.include_time_consuming_objectives,
self.excluded_games_time_consuming_objectives,
self.game_medley_mode,
self.game_medley_percentage_chance,
self.game_medley_game_selection_bag_size,
self.objective_selection_bag_size,
)
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call to generate_from_plan is now passing 12 positional arguments, which is easy to mis-order as the signature evolves (as happened here when adding objective_selection_bag_size). Prefer passing these by keyword for readability and to prevent subtle parameter ordering bugs.

Copilot uses AI. Check for mistakes.
Comment on lines 152 to 154
objectives.append(objective)
objectives_in_use[objective] = objectives_in_use.get(objective, 0) + 1
break
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The passes_templates > 50 fallback currently appends an objective even if objective_bag_size has already been reached for that objective, which means the configured repeat cap can be violated (including when set to 1). Consider making the fallback still honor objective_bag_size (e.g., by failing with a helpful error) so the option’s behavior is deterministic.

Suggested change
objectives.append(objective)
objectives_in_use[objective] = objectives_in_use.get(objective, 0) + 1
break
raise RuntimeError(
"Unable to generate additional unique objectives without exceeding the "
"configured objective_bag_size. Consider reducing 'count' or increasing "
"'objective_bag_size'."
)

Copilot uses AI. Check for mistakes.
Refine the ObjectiveSelectionBagSize docstring to use firmer wording ("should" instead of "can") and add a note that the global cap is best-effort
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