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

[Feature / Cleanup] Sign rework (WIP) #679

Open
wants to merge 123 commits into
base: main
Choose a base branch
from

Conversation

DerToaster98
Copy link
Contributor

@DerToaster98 DerToaster98 commented Aug 1, 2024

Describe in detail what your pull request accomplishes

This PR is a attempt at redoing signs in a more organized way.
That encompasses the following things:

  • Signs get registered to a central registry / map
  • A single listener retrieves the fitting registered sign and calls those for processing
  • Signs can be overridden, useful for addons that want to change behaviors of let's say move signs (e.g. Squadrons reloaded)
  • Removes redundant code and should make it cleaner
  • Should make remote sign implementations much cleaner cause that way one can just call the relevant registered sign instead of firing a new event

TODO List:

  • Base class for signs
  • Base class for "craft signs" (everything that requires a craft and that can react to craft detection and sign translation)
  • Sign registration
  • Sign retrieval by "ID"
  • Base cruise sign
  • Base information sign
  • AbstractCruiseSign => will be the base class for signs acting similar to the cruise sign
  • Abstraction layer for signs
  • 1.20+: Support for sided signs
  • Deprecation notice and comment in all relevant places that use the SignWrapper as discussedin Change SignTranslateEvent to use Adventure #688
  • Change SignTranslateEvent to use the SignWrapper
  • Subcraft base sign
  • Re-Implement existing signs
    • Ascend sign
    • Descend sign
    • Cruise sign
    • Helm sign
    • Name sign
    • Move sign
    • Pilot sign => TODO: Should we replace the PilotValidator in the detection task or not?
    • RelativeMove sign
    • Release sign
    • Remote sign => will receive rework to better handle multiple target signs and to avoid beavering => Not possible without rewriting how the release stuff works
    • Scuttle sign
    • Speed sign
    • SubcraftRotate => will maybe receive a rework so it is more reliable and less prone to beavering
    • Teleport sign
    • Status sign
    • Contacts sign
    • [ ] NEW Subcraft Move Postpone for later, this requires thorough analysis prior

Checklist

  • Proper internationalization
  • Tested
  • Performance tested (could improve performance on remote signs and subcraft, not done yet)

@DerToaster98 DerToaster98 changed the title [Feature / Cleanup] Sign rework [Feature / Cleanup] Sign rework (WIP) Aug 1, 2024
@TylerS1066
Copy link
Contributor

  * [ ]  **NEW** Subcraft Move

This would be best left for a separate PR. I believe there are many assumptions that Subcrafts never move, so we'll have to do a through review of the codebase for that feature.

@DerToaster98
Copy link
Contributor Author

  * [ ]  **NEW** Subcraft Move

This would be best left for a separate PR. I believe there are many assumptions that Subcrafts never move, so we'll have to do a through review of the codebase for that feature.

Alright, i will remove that from the todo list. Will implement the other points nonetheless

(cherry picked from commit ce7f379)
(cherry picked from commit 719a1ec)
(cherry picked from commit 7075208)
(cherry picked from commit 1aaf852)
(cherry picked from commit 956c8c5)
(cherry picked from commit d70b65b)
(cherry picked from commit 8e1e7c7)
(cherry picked from commit e2bae7d)
(cherry picked from commit 4eda66f)
(cherry picked from commit 3194ed7)
(cherry picked from commit 19e2171)
(cherry picked from commit 2df0385)
(cherry picked from commit 6d8b47a)
(cherry picked from commit 9a40431)
(cherry picked from commit 232ab10)
(cherry picked from commit 91ff872)
(cherry picked from commit 3aa0dc9)
(cherry picked from commit 88baa63)
(cherry picked from commit 2316876)
(cherry picked from commit 238e989)
(cherry picked from commit fd70b50)
(cherry picked from commit 17b458e)
(cherry picked from commit 21c4554)
(cherry picked from commit c2978b0)
(cherry picked from commit c83b41e)
(cherry picked from commit 9d850ce)
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