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

Craft DataTags and trackedlocations #611

Closed

Conversation

ccorp2002
Copy link
Contributor

@ccorp2002 ccorp2002 commented Sep 26, 2023

Describe in detail what your pull request accomplishes

Craft DataTags:
Useful for storing any type of Data in Key-Value format, "aboard" a Craft Object.
As long as the Craft Object isn't erased/lost-track-of, the DataTags will remain.

Example Use-Case:
Storing a List of Players who can Activate Certain Types of Blocks on a craft.
Keeping track of the Number of Damaging Hits Recieved and

TrackedLocations:
A MovecraftLocation and/or HitBox-Object that will move "with" the Craft it is bound to, when the Craft Translates or Rotates

Example Use-Case:
A Better "Movecraft-Cannons" Addon-Plugin.
Where the Cannon's Locations are automatically Updated/Moved with the Craft upon any successful Movement/Rotation.

Checklist

  • Proper internationalization (N/A)
  • Tested (I have been using my Private Version of this for like a year or two now.)

Craft DataTags: Useful for storing any type of Data in Key-Value format, "aboard" a Craft Object.

As long as the Craft Object isn't erased/lost-track-of, the DataTags will remain.

TrackedLocations: A MovecraftLocation and/or HitBox-Object that will move "with" the Craft it is bound to, when the Craft Translates or Rotates

Example Use-Case: A Better "Movecraft-Cannons" Plugin.
Where the Cannon's Locations are automatically Updated/Moved with the Craft upon any successful Movement.
Added Implementation of Craft DataTags & Tracked Locations

Craft DataTags: Useful for storing any type of Data in Key-Value format, "aboard" a Craft Object.

As long as the Craft Object isn't erased/lost-track-of, the DataTags will remain.

TrackedLocations: A MovecraftLocation and/or HitBox-Object that will move "with" the Craft it is bound to, when the Craft Translates or Rotates

Example Use-Case: A Better "Movecraft-Cannons" Plugin.
Where the Cannon's Locations are automatically Updated/Moved with the Craft upon any successful Movement.
Forgot Nullable
Chaos Chaos!!!
Made Craft Un-Based
A temporary file to prove that it compiles (hopefully)
Forgot to add the copy of locations etc
Copy link
Contributor

@TylerS1066 TylerS1066 left a comment

Choose a reason for hiding this comment

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

Just some initial review comments:

  1. Let's split this PR into two items: Tracked Locations and Data Tags. I know they are similar, but it'd be easier to separate them and individually address issues in them.

  2. Data Tag comments:
    a. We'll want to use a NamespacedKey similar to the CraftType's property system. The benefit is that the hash of NamespacedKeys are computed at build time, rather than during runtime.
    b. We don't need so many functions for data tags, only access to set (null being remove) and get (a null being does not exist).
    c. I'm not sure if we want to do it here, but for the property system we had added type safety by using a number of maps, but the benefit there not only for type safety but also to utilize the same config loading code. Since these values aren't going to be derived from a file, perhaps the type safety isn't worth the complexity? Alternatively, we could look at implementing something like how Spigot does PersistentDataContainers.

  3. Tracked locations comments:
    a. Craft#translateBox/rotateBox should be combined into Craft#translate/rotate
    b. Instead of moving each MovecraftLocation tracked, we should instead keep track of an "origin" of the craft, then store these as offsets from the origin. We can add some simple functions to calculate the actual MovecraftLocation from a tracked location when needed, but I expect we'll have more movements/rotations than queries of the tracked locations. The benefit of this is that no processing is required on translation, and we only need to use the highly optimized rotation math on a rotation. Add-ons may need to query the world location of each tracked location on each movement, but for those that don't we save processing time.
    c. I'm not fully sure how we want to do it, but we'll want to use a similar key-value system for keeping track of the TrackedLocations. My idea is that we would let add-ons create a NamespacedKey for their TrackedLocations, so each craft would have a Map<NamespacedKey,Set<TrackedLocation>>. That way one add-on could register their set of tracked locations and modify the set as they wish, while Movecraft automagically moves them. Meanwhile, another add-on could register their own set of tracked locations, and handle them as they wish. The benefit here, is that if a plugin wishes to keep more data with a TrackedLocation, they can merely inherit from it and add more fields, while the base class will just store the offset.

@ccorp2002
Copy link
Contributor Author

ccorp2002 commented Sep 27, 2023

I have no real functional experience with Git & GitHub, so I don't really know how I'd split the PR in two, though, I agree.

My idea for Data Tags was a simple way to store any type of data "aboard" a Craft Object, while in existence, and you could keep track of a variety of things with that.
Ex: You could keep tally of the names of other ships who you've sunk; or the different pilots you have blown up, etc.
I feel that, while using a namespace key makes sense from a Developer's Perspective, who has worked with the internals of plugins a lot. But for those just starting to get into Movecraft and integrating it into other plugins and systems, personally, I think it should remain how it is

Personally I think translateBox/rotateBox probably should stay the same.
translateBox and rotateBox are a simple shortcut for rotating/moving HitBoxes independently of a craft's movements, not just a Collection of Tracked MovecraftLocations/HitBoxes.

While it is used for moving/rotating Tracked MovecraftLocations/HitBoxes in the MapUpdater section of my changes, that's not all I have used it for on my end.

I can see a variety of uses such as tracking a Subcraft's Hitbox aboard a Craft or even not physically attached to a Craft, and rotating that relative to the Parent Craft, or rotating/translating a Hitbox without being bound to a Craft.
Hitboxes can be useful for other things than just determining a Craft's Bounds, and for me, at least, is useful for tracking a multiblock or set of disconnected-multiblocks aboard ships.

However, as for an actual TrackedLocation Object being created, I agree, but I still think using the non-relative MovecraftLocation should still be an option for those who just want a simple way of keeping track of coordinates.

@DerToaster98 DerToaster98 mentioned this pull request Jul 21, 2024
3 tasks
@TylerS1066
Copy link
Contributor

This PR has been superseded by #616 and #673.

@TylerS1066 TylerS1066 closed this Jul 28, 2024
@ccorp2002 ccorp2002 deleted the datatags-and-trackedlocations branch November 13, 2024 07:13
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.

2 participants