From f62d9bc0496a0f2cfb82268bc3da756a85679357 Mon Sep 17 00:00:00 2001 From: massimilianodelliubaldini <8584296+massimilianodelliubaldini@users.noreply.github.com> Date: Sat, 5 Apr 2025 21:00:15 -0400 Subject: [PATCH] First round of Treble code reviews. --- worlds/jakanddaxter/Items.py | 12 ++-- worlds/jakanddaxter/Locations.py | 72 +++++++++++----------- worlds/jakanddaxter/Options.py | 15 ++--- worlds/jakanddaxter/Rules.py | 2 +- worlds/jakanddaxter/__init__.py | 37 +++++------ worlds/jakanddaxter/test/test_moverando.py | 2 +- 6 files changed, 69 insertions(+), 71 deletions(-) diff --git a/worlds/jakanddaxter/Items.py b/worlds/jakanddaxter/Items.py index 662ec2b715..7d72fc433f 100644 --- a/worlds/jakanddaxter/Items.py +++ b/worlds/jakanddaxter/Items.py @@ -122,11 +122,11 @@ trap_item_table = { # All Items # While we're here, do all the ID conversions needed. item_table = { - **{Cells.to_ap_id(k): cell_item_table[k] for k in cell_item_table}, - **{Scouts.to_ap_id(k): scout_item_table[k] for k in scout_item_table}, - **{Specials.to_ap_id(k): special_item_table[k] for k in special_item_table}, - **{Caches.to_ap_id(k): move_item_table[k] for k in move_item_table}, - **{Orbs.to_ap_id(k): orb_item_table[k] for k in orb_item_table}, - **{jak1_max - k: trap_item_table[k] for k in trap_item_table}, + **{Cells.to_ap_id(k): name for k, name in cell_item_table.items()}, + **{Scouts.to_ap_id(k): name for k, name in scout_item_table.items()}, + **{Specials.to_ap_id(k): name for k, name in special_item_table.items()}, + **{Caches.to_ap_id(k): name for k, name in move_item_table.items()}, + **{Orbs.to_ap_id(k): name for k, name in orb_item_table.items()}, + **{jak1_max - k: name for k, name in trap_item_table.items()}, jak1_max: "Green Eco Pill" # Filler item. } diff --git a/worlds/jakanddaxter/Locations.py b/worlds/jakanddaxter/Locations.py index 98517aa449..501a864316 100644 --- a/worlds/jakanddaxter/Locations.py +++ b/worlds/jakanddaxter/Locations.py @@ -21,47 +21,47 @@ class JakAndDaxterLocation(Location): # Different tables for location groups. # Each Item ID == its corresponding Location ID. While we're here, do all the ID conversions needed. cell_location_table = { - **{Cells.to_ap_id(k): Cells.loc7SF_cellTable[k] for k in Cells.loc7SF_cellTable}, - **{Cells.to_ap_id(k): Cells.locGR_cellTable[k] for k in Cells.locGR_cellTable}, - **{Cells.to_ap_id(k): Cells.locSV_cellTable[k] for k in Cells.locSV_cellTable}, - **{Cells.to_ap_id(k): Cells.locFJ_cellTable[k] for k in Cells.locFJ_cellTable}, - **{Cells.to_ap_id(k): Cells.locSB_cellTable[k] for k in Cells.locSB_cellTable}, - **{Cells.to_ap_id(k): Cells.locMI_cellTable[k] for k in Cells.locMI_cellTable}, - **{Cells.to_ap_id(k): Cells.locFC_cellTable[k] for k in Cells.locFC_cellTable}, - **{Cells.to_ap_id(k): Cells.locRV_cellTable[k] for k in Cells.locRV_cellTable}, - **{Cells.to_ap_id(k): Cells.locPB_cellTable[k] for k in Cells.locPB_cellTable}, - **{Cells.to_ap_id(k): Cells.locLPC_cellTable[k] for k in Cells.locLPC_cellTable}, - **{Cells.to_ap_id(k): Cells.locBS_cellTable[k] for k in Cells.locBS_cellTable}, - **{Cells.to_ap_id(k): Cells.locMP_cellTable[k] for k in Cells.locMP_cellTable}, - **{Cells.to_ap_id(k): Cells.locVC_cellTable[k] for k in Cells.locVC_cellTable}, - **{Cells.to_ap_id(k): Cells.locSC_cellTable[k] for k in Cells.locSC_cellTable}, - **{Cells.to_ap_id(k): Cells.locSM_cellTable[k] for k in Cells.locSM_cellTable}, - **{Cells.to_ap_id(k): Cells.locLT_cellTable[k] for k in Cells.locLT_cellTable}, - **{Cells.to_ap_id(k): Cells.locGMC_cellTable[k] for k in Cells.locGMC_cellTable} + **{Cells.to_ap_id(k): name for k, name in Cells.loc7SF_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locGR_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locSV_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locFJ_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locSB_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locMI_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locFC_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locRV_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locPB_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locLPC_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locBS_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locMP_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locVC_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locSC_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locSM_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locLT_cellTable.items()}, + **{Cells.to_ap_id(k): name for k, name in Cells.locGMC_cellTable.items()}, } scout_location_table = { - **{Scouts.to_ap_id(k): Scouts.locGR_scoutTable[k] for k in Scouts.locGR_scoutTable}, - **{Scouts.to_ap_id(k): Scouts.locSV_scoutTable[k] for k in Scouts.locSV_scoutTable}, - **{Scouts.to_ap_id(k): Scouts.locFJ_scoutTable[k] for k in Scouts.locFJ_scoutTable}, - **{Scouts.to_ap_id(k): Scouts.locSB_scoutTable[k] for k in Scouts.locSB_scoutTable}, - **{Scouts.to_ap_id(k): Scouts.locMI_scoutTable[k] for k in Scouts.locMI_scoutTable}, - **{Scouts.to_ap_id(k): Scouts.locFC_scoutTable[k] for k in Scouts.locFC_scoutTable}, - **{Scouts.to_ap_id(k): Scouts.locRV_scoutTable[k] for k in Scouts.locRV_scoutTable}, - **{Scouts.to_ap_id(k): Scouts.locPB_scoutTable[k] for k in Scouts.locPB_scoutTable}, - **{Scouts.to_ap_id(k): Scouts.locLPC_scoutTable[k] for k in Scouts.locLPC_scoutTable}, - **{Scouts.to_ap_id(k): Scouts.locBS_scoutTable[k] for k in Scouts.locBS_scoutTable}, - **{Scouts.to_ap_id(k): Scouts.locMP_scoutTable[k] for k in Scouts.locMP_scoutTable}, - **{Scouts.to_ap_id(k): Scouts.locVC_scoutTable[k] for k in Scouts.locVC_scoutTable}, - **{Scouts.to_ap_id(k): Scouts.locSC_scoutTable[k] for k in Scouts.locSC_scoutTable}, - **{Scouts.to_ap_id(k): Scouts.locSM_scoutTable[k] for k in Scouts.locSM_scoutTable}, - **{Scouts.to_ap_id(k): Scouts.locLT_scoutTable[k] for k in Scouts.locLT_scoutTable}, - **{Scouts.to_ap_id(k): Scouts.locGMC_scoutTable[k] for k in Scouts.locGMC_scoutTable} + **{Scouts.to_ap_id(k): name for k, name in Scouts.locGR_scoutTable.items()}, + **{Scouts.to_ap_id(k): name for k, name in Scouts.locSV_scoutTable.items()}, + **{Scouts.to_ap_id(k): name for k, name in Scouts.locFJ_scoutTable.items()}, + **{Scouts.to_ap_id(k): name for k, name in Scouts.locSB_scoutTable.items()}, + **{Scouts.to_ap_id(k): name for k, name in Scouts.locMI_scoutTable.items()}, + **{Scouts.to_ap_id(k): name for k, name in Scouts.locFC_scoutTable.items()}, + **{Scouts.to_ap_id(k): name for k, name in Scouts.locRV_scoutTable.items()}, + **{Scouts.to_ap_id(k): name for k, name in Scouts.locPB_scoutTable.items()}, + **{Scouts.to_ap_id(k): name for k, name in Scouts.locLPC_scoutTable.items()}, + **{Scouts.to_ap_id(k): name for k, name in Scouts.locBS_scoutTable.items()}, + **{Scouts.to_ap_id(k): name for k, name in Scouts.locMP_scoutTable.items()}, + **{Scouts.to_ap_id(k): name for k, name in Scouts.locVC_scoutTable.items()}, + **{Scouts.to_ap_id(k): name for k, name in Scouts.locSC_scoutTable.items()}, + **{Scouts.to_ap_id(k): name for k, name in Scouts.locSM_scoutTable.items()}, + **{Scouts.to_ap_id(k): name for k, name in Scouts.locLT_scoutTable.items()}, + **{Scouts.to_ap_id(k): name for k, name in Scouts.locGMC_scoutTable.items()}, } -special_location_table = {Specials.to_ap_id(k): Specials.loc_specialTable[k] for k in Specials.loc_specialTable} -cache_location_table = {Caches.to_ap_id(k): Caches.loc_orbCacheTable[k] for k in Caches.loc_orbCacheTable} -orb_location_table = {Orbs.to_ap_id(k): Orbs.loc_orbBundleTable[k] for k in Orbs.loc_orbBundleTable} +special_location_table = {Specials.to_ap_id(k): name for k, name in Specials.loc_specialTable.items()} +cache_location_table = {Caches.to_ap_id(k): name for k, name in Caches.loc_orbCacheTable.items()} +orb_location_table = {Orbs.to_ap_id(k): name for k, name in Orbs.loc_orbBundleTable.items()} # All Locations location_table = { diff --git a/worlds/jakanddaxter/Options.py b/worlds/jakanddaxter/Options.py index 55aa11271c..421b0f28af 100644 --- a/worlds/jakanddaxter/Options.py +++ b/worlds/jakanddaxter/Options.py @@ -16,7 +16,8 @@ class EnableOrbsanity(Choice): """Include bundles of Precursor Orbs as checks. Every time you collect the chosen number of orbs, you will trigger another check. - "Per Level" means bundles are for each level in the game. "Global" means bundles carry over level to level. + Per Level: bundles are for each level in the game. + Global: bundles carry over level to level. This adds a number of Items and Locations to the pool inversely proportional to the size of the bundle. For example, if your bundle size is 20 orbs, you will add 100 items to the pool. If your bundle size is 250 orbs, @@ -29,7 +30,7 @@ class EnableOrbsanity(Choice): class GlobalOrbsanityBundleSize(Choice): - """Set the orb bundle size for Global Orbsanity. This only applies if "Enable Orbsanity" is set to "Global." + """The orb bundle size for Global Orbsanity. This only applies if "Enable Orbsanity" is set to "Global." There are 2000 orbs in the game, so your bundle size must be a factor of 2000. Multiplayer Minimum: 10 @@ -61,7 +62,7 @@ class GlobalOrbsanityBundleSize(Choice): class PerLevelOrbsanityBundleSize(Choice): - """Set the orb bundle size for Per Level Orbsanity. This only applies if "Enable Orbsanity" is set to "Per Level." + """The orb bundle size for Per Level Orbsanity. This only applies if "Enable Orbsanity" is set to "Per Level." There are 50, 150, or 200 orbs per level, so your bundle size must be a factor of 50. Multiplayer Minimum: 10""" @@ -77,7 +78,7 @@ class PerLevelOrbsanityBundleSize(Choice): class FireCanyonCellCount(Range): - """Set the number of power cells you need to cross Fire Canyon. + """The number of power cells you need to cross Fire Canyon. Multiplayer Maximum: 30 Singleplayer Maximum: 34""" @@ -90,7 +91,7 @@ class FireCanyonCellCount(Range): class MountainPassCellCount(Range): - """Set the number of power cells you need to reach Klaww and cross Mountain Pass. + """The number of power cells you need to reach Klaww and cross Mountain Pass. Multiplayer Maximum: 60 Singleplayer Maximum: 63""" @@ -103,7 +104,7 @@ class MountainPassCellCount(Range): class LavaTubeCellCount(Range): - """Set the number of power cells you need to cross Lava Tube. + """The number of power cells you need to cross Lava Tube. Multiplayer Maximum: 90 Singleplayer Maximum: 99""" @@ -116,7 +117,7 @@ class LavaTubeCellCount(Range): class EnableOrderedCellCounts(DefaultOnToggle): - """Reorders the Cell Count requirements for vehicle sections to be in ascending order. + """Reorder the Cell Count requirements for vehicle sections to be in ascending order. For example, if Fire Canyon Cell Count, Mountain Pass Cell Count, and Lava Tube Cell Count are 60, 30, and 40 respectively, they will be reordered to 30, 40, and 60.""" diff --git a/worlds/jakanddaxter/Rules.py b/worlds/jakanddaxter/Rules.py index 437c2cca8c..baae22c772 100644 --- a/worlds/jakanddaxter/Rules.py +++ b/worlds/jakanddaxter/Rules.py @@ -120,7 +120,7 @@ def can_free_scout_flies(state: CollectionState, player: int) -> bool: def can_fight(state: CollectionState, player: int) -> bool: - return state.has_any({"Jump Dive", "Jump Kick", "Punch", "Kick"}, player) + return state.has_any(("Jump Dive", "Jump Kick", "Punch", "Kick"), player) def enforce_multiplayer_limits(world: JakAndDaxterWorld): diff --git a/worlds/jakanddaxter/__init__.py b/worlds/jakanddaxter/__init__.py index ec79c7f42a..289320dae6 100644 --- a/worlds/jakanddaxter/__init__.py +++ b/worlds/jakanddaxter/__init__.py @@ -139,8 +139,8 @@ class JakAndDaxterWorld(World): # Stored as {ID: Name} pairs, these must now be swapped to {Name: ID} pairs. # Remember, the game ID and various offsets for each item type have already been calculated. - item_name_to_id = {item_table[k]: k for k in item_table} - location_name_to_id = {location_table[k]: k for k in location_table} + item_name_to_id = {name: k for k, name in item_table.items()} + location_name_to_id = {name: k for k, name in location_table.items()} item_name_groups = { "Power Cells": set(cell_item_table.values()), "Scout Flies": set(scout_item_table.values()), @@ -222,34 +222,31 @@ class JakAndDaxterWorld(World): total_prog_cells: int = 0 total_trap_cells: int = 0 total_filler_cells: int = 0 - power_cell_thresholds: list[int] = [] - trap_weights: tuple[list[str], list[int]] = ({}, {}) + power_cell_thresholds: list[int] + trap_weights: tuple[list[str], list[int]] # Handles various options validation, rules enforcement, and caching of important information. def generate_early(self) -> None: # Cache the power cell threshold values for quicker reference. - self.power_cell_thresholds = [] - self.power_cell_thresholds.append(self.options.fire_canyon_cell_count.value) - self.power_cell_thresholds.append(self.options.mountain_pass_cell_count.value) - self.power_cell_thresholds.append(self.options.lava_tube_cell_count.value) - self.power_cell_thresholds.append(100) # The 100 Power Cell Door. + self.power_cell_thresholds = [ + self.options.fire_canyon_cell_count.value, + self.options.mountain_pass_cell_count.value, + self.options.lava_tube_cell_count.value, + 100, # The 100 Power Cell Door. + ] # Order the thresholds ascending and set the options values to the new order. - try: - if self.options.enable_ordered_cell_counts: - self.power_cell_thresholds.sort() - self.options.fire_canyon_cell_count.value = self.power_cell_thresholds[0] - self.options.mountain_pass_cell_count.value = self.power_cell_thresholds[1] - self.options.lava_tube_cell_count.value = self.power_cell_thresholds[2] - except IndexError: - pass # Skip if not possible. + if self.options.enable_ordered_cell_counts: + self.power_cell_thresholds.sort() + self.options.fire_canyon_cell_count.value = self.power_cell_thresholds[0] + self.options.mountain_pass_cell_count.value = self.power_cell_thresholds[1] + self.options.lava_tube_cell_count.value = self.power_cell_thresholds[2] # For the fairness of other players in a multiworld game, enforce some friendly limitations on our options, # so we don't cause chaos during seed generation. These friendly limits should **guarantee** a successful gen. # We would have done this earlier, but we needed to sort the power cell thresholds first. - ap_settings = settings.get_settings() - enforce_friendly_options = ap_settings.jakanddaxter_options.enforce_friendly_options + enforce_friendly_options = self.settings.enforce_friendly_options if enforce_friendly_options: if self.multiworld.players > 1: from .Rules import enforce_multiplayer_limits @@ -395,7 +392,7 @@ class JakAndDaxterWorld(World): # Manually fill the item pool with a weighted assortment of trap items, equal to the sum of # total_trap_cells + total_trap_orb_bundles. Only do this if one or more traps have weights > 0. names, weights = self.trap_weights - if sum(weights) > 0: + if sum(weights): total_traps = self.total_trap_cells + self.total_trap_orb_bundles trap_list = self.random.choices(names, weights=weights, k=total_traps) self.multiworld.itempool += [self.create_item(trap_name) for trap_name in trap_list] diff --git a/worlds/jakanddaxter/test/test_moverando.py b/worlds/jakanddaxter/test/test_moverando.py index 10c8a0ca98..4e9dcf8847 100644 --- a/worlds/jakanddaxter/test/test_moverando.py +++ b/worlds/jakanddaxter/test/test_moverando.py @@ -29,4 +29,4 @@ class NoMoveRandoTest(JakAndDaxterTestBase): for move in move_item_table: self.assertNotIn(move_item_table[move], {item.name for item in self.multiworld.itempool}) self.assertIn(move_item_table[move], - {item.name for item in self.multiworld.precollected_items[self.player]}) \ No newline at end of file + {item.name for item in self.multiworld.precollected_items[self.player]})