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

Add generic multiworld tracker, move lttp multiworld tracker #1478

Merged
merged 5 commits into from
Mar 8, 2023

Conversation

freyacodes
Copy link
Contributor

@freyacodes freyacodes commented Feb 23, 2023

(Rewritten because of change in scope)

This relates to the discussion in #1477

What is this fixing or adding?

  • Move existing multitracker to a new path
  • Add a new generic multiworld tracker based on the original
  • Add a dynamic navigation menu

The navigation menu can support multiple multitrackers. It will only appear if there are games present with a specific multitracker available (or if you navigated to one of the specific ones).

How was this tested?

This has been tested by opening the multiworld trackers with and without ALttP worlds. No gameplay was attempted. This PR is based on the latest revision of main as of this writing.

If this makes graphical changes, please attach screenshots.

Screenshots

image
image

@Berserker66
Copy link
Member

Commented on the issue why this will not be merged in its current incarnation.

@freyacodes freyacodes changed the title Remove AlttP elements from the multiworld tracker Add generic multiworld tracker, move lttp multiworld tracker Feb 24, 2023
@freyacodes freyacodes marked this pull request as draft February 24, 2023 10:27
@freyacodes freyacodes marked this pull request as ready for review February 24, 2023 15:13
@LegendaryLinux
Copy link
Member

As mentioned in #1477, I think the LttP specific code should be removed, not embraced.

@Berserker66
Copy link
Member

This now has conflicts in the js file

Copy link
Member

@Berserker66 Berserker66 left a comment

Choose a reason for hiding this comment

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

cleared the conflict, fixed LttP not tracking at all, fixed pep8-ness, tested.
Since this now includes my code, it should be reviewed and tested by another maintainer though.

@LegendaryLinux
Copy link
Member

I'll tinker with this sometime this week. I think the answer is to adjust the header to display a hamburger menu when the screen layout is taller than it is shorter, and have that button be clickable to reveal the menu.

@Berserker66
Copy link
Member

I'll merge it for now, as I want to test it on the test server and it should be functional. Obviously feel free to PR in style upgrades later.
(Currently the tracker is LttP only and broken, so this PR is a clear upgrade)

@Berserker66 Berserker66 merged commit a95e51d into ArchipelagoMW:main Mar 8, 2023
@freyacodes freyacodes deleted the stripped-tracker branch March 8, 2023 23:22
kindasneaki pushed a commit to kindasneaki/Archipelago that referenced this pull request Jun 28, 2023
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
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