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

Factorio, WebHost: fix website multitracker #2126

Merged
merged 4 commits into from
Sep 9, 2023
Merged

Conversation

Berserker66
Copy link
Member

What is this fixing or adding?

https://discord.com/channels/731205301247803413/1145166962910040104

How was this tested?

minimally

If this makes graphical changes, please attach screenshots.

WebHostLib/tracker.py Outdated Show resolved Hide resolved
Co-authored-by: Remy Jette <remy@remyjette.com>
@remyjette
Copy link
Collaborator

I just tested it with progressive techs on and off, seems to be working correctly now. Thanks!

Copy link
Member

@black-sliver black-sliver left a comment

Choose a reason for hiding this comment

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

This should work for current seeds, however if we ever change IDs again, this will break. item_name_to_id really should be fetched from the actual games package the multidata uses.

@Berserker66
Copy link
Member Author

yes

@black-sliver
Copy link
Member

yes

so, do you want to merge this as-is or fix it in one swoop?

@Berserker66
Copy link
Member Author

I would prefer to fix that too, if I can find the time to do so.

Copy link
Member

@black-sliver black-sliver left a comment

Choose a reason for hiding this comment

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

Looks fine to me, however I have not tested it. Maybe someone else can?

@@ -1436,6 +1439,17 @@ def _get_inventory_data(data: typing.Dict[str, typing.Any]) -> typing.Dict[int,
return inventory


def _get_named_inventory(inventory: typing.Dict[int, int], custom_items: typing.Dict[int, str] = None) \
-> typing.Dict[str, int]:
"""slow"""
Copy link
Member

Choose a reason for hiding this comment

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

Could probably be lazy?

@black-sliver
Copy link
Member

Also you are not passing the custom_items in yet. I'm assuming that's a future plan?

@Berserker66
Copy link
Member Author

nope, forgotten to do

@remyjette
Copy link
Collaborator

remyjette commented Sep 6, 2023

Looks fine to me, however I have not tested it. Maybe someone else can?

I just tested it and things appear to be working correctly. I just tested current IDs, not what happens if IDs change, but its working at least if I generate a game on main with this patch.

@Berserker66 Berserker66 merged commit 29f8053 into main Sep 9, 2023
20 checks passed
@Berserker66 Berserker66 deleted the factorio_multitracker branch September 9, 2023 22:33
@ThePhar ThePhar added is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. affects: webhost Issues/PRs that touch webhost and may need additional validation. labels Oct 16, 2023
@ThePhar ThePhar changed the title Factorio: fix website multitracker Factorio, WebHost: fix website multitracker Oct 16, 2023
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
Co-authored-by: Remy Jette <remy@remyjette.com>
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
Co-authored-by: Remy Jette <remy@remyjette.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: webhost Issues/PRs that touch webhost 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.

None yet

4 participants