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

Waypoints refactor #976

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

kevinthegreat1
Copy link
Collaborator

@kevinthegreat1 kevinthegreat1 commented Sep 1, 2024

Highlights

Ordered waypoints have been migrated to waypoints gui.

  • All ordered waypoints made with commands will be migrated to waypoints.
  • All commands have been removed except for importing the Skyblocker Ordered Waypoints format, which will be immediately migrated to waypoints gui once imported.
  • Skyblocker Ordered Waypoints format is now unused.

New Skyblocker Waypoints format

  • Similar to the existing Skyblocker Ordered Waypoints format but more extensible.

API Design

The "wither" pattern is employed. Each class extending Waypoint should override all "wither" methods to return a new instance of the subclass to ensure the runtime type of the waypoint is correct.

API Changes

I have tried to keep API changes to a minimum, but here are the breaking changes:

  • Waypoint#setShouldRender has been renamed to Waypoint#setEnabled.
  • Waypoint#getColorComponents has been renamed to Waypoint#getRenderColorComponents.
  • Waypoint#render now checks Waypoint#shouldRender.
    • This allows only rendering the name by overriding shouldRender to false and shouldRenderName to true.
  • NamedWaypoint#shouldRenderName now defaults to Waypoint#shouldRender.
    • You now need to override shouldRenderName to always render the name.

Bug Fixes

@LifeIsAParadox LifeIsAParadox added the reviews needed This PR needs reviews label Sep 1, 2024
@AzureAaron AzureAaron added this to the 1.23.0 milestone Sep 1, 2024
viciscat
viciscat previously approved these changes Sep 17, 2024
Copy link
Collaborator

@viciscat viciscat left a comment

Choose a reason for hiding this comment

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

Looks good I suppose but did not test, might do that at some point.

@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels Sep 17, 2024
@kevinthegreat1
Copy link
Collaborator Author

I’ll wait for aaron to look at this pr.

@kevinthegreat1 kevinthegreat1 added reviews needed This PR needs reviews and removed merge me please Pull requests that are ready to merge labels Sep 20, 2024
@LifeIsAParadox LifeIsAParadox added changes requested This PR need changes and removed reviews needed This PR needs reviews labels Sep 21, 2024
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed changes requested This PR need changes labels Sep 21, 2024
@kevinthegreat1
Copy link
Collaborator Author

Closes #1024.

@LifeIsAParadox LifeIsAParadox added merge conflicts This PR has merge conflicts that need solving. and removed reviews needed This PR needs reviews labels Nov 16, 2024
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed merge conflicts This PR has merge conflicts that need solving. labels Nov 22, 2024
@LifeIsAParadox LifeIsAParadox added changes requested This PR need changes reviews needed This PR needs reviews and removed reviews needed This PR needs reviews changes requested This PR need changes labels Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reviews needed This PR needs reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

waypoints not working
4 participants