Skip to content
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

Core: remove start_inventory_from_pool from early_items #2579

Merged
merged 2 commits into from
Dec 10, 2023

Conversation

Berserker66
Copy link
Member

@Berserker66 Berserker66 commented Dec 8, 2023

What is this fixing or adding?

Exhausts start_inventory_from_pool items from early_items as well, which fixes:

  1. Cases where say early Seaglide has 2 early items, but player could start with 4, making AP search for them to place early but never finding them, wasting time. I believe this would also crash SDV.
  2. Cases where too much would be early. Early Seaglide in Subnautica places 2 Fragments out of 4 into sphere 1, if you have 2 as start_inventory_from_pool you actually have 2 start and 2 in sphere 1, which technically means 4 "early", when 2 was desired.

How was this tested?

With Subnautica Seaglide Fragments

If this makes graphical changes, please attach screenshots.

@@ -117,6 +117,15 @@ def main(args, seed=None, baked_server_options: Optional[Dict[str, object]] = No
for item_name, count in world.start_inventory_from_pool.setdefault(player, StartInventoryPool({})).value.items():
for _ in range(count):
world.push_precollected(world.create_item(item_name, player))
# remove from_pool items also from early items handling, as starting is plenty early.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this snippet would be way easier to read and understand with slightly improved variable names. For example, "early" is used twice, in mutually exclusive usages (assign, then read, then assign again, then read again), so if it was two variables, making the difference between both usages more clear, it would be a good improvement

@ScootyPuffJr1 ScootyPuffJr1 added the affects: core Issues/PRs that touch core and may need additional validation. label Dec 9, 2023
@Berserker66 Berserker66 added the is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. label Dec 9, 2023
Main.py Outdated Show resolved Hide resolved
@Berserker66 Berserker66 merged commit 13122ab into main Dec 10, 2023
20 of 21 checks passed
@Berserker66 Berserker66 deleted the core_remove_early_items_by_start_pool branch December 10, 2023 19:42
Magnemania pushed a commit to Magnemania/Archipelago that referenced this pull request Jan 3, 2024
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants