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

Dynamic Craft Variables #612

Closed
goodroach opened this issue Oct 4, 2023 · 3 comments
Closed

Dynamic Craft Variables #612

goodroach opened this issue Oct 4, 2023 · 3 comments

Comments

@goodroach
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Currently, there is no way to dynamically adjust craft variables such as speed, block percentages, etc. This means any dynamically changing variable such as speed implemented requires a drastic change to Movecraft's base code. I believe this is one of Movecraft's greatest constraints when developing add-ons for Movecraft, as I am not able to adjust variables as I please while the craft is being piloted.

As of current, there are not many dynamically changing variables in a craft. As time goes on, this will surely increase to the point where movecraft will bloat with features just to interact with its current speed variable or other craft-related variables.

Describe the solution you'd like
I believe the solution for this is to completely refactor how movecraft interacts with the craft objects by having all the data including all the variables in the BaseCraft and CraftType objects to be included in a single hashmap indexed by NameSpacedKey. This will be very similar to how CraftType records all the individual variables for each craft type. However, in this case, it will be for individual crafts.

Additional notes
I am aware of the difficulty of this task, but I believe this will be a great long-term solution for movecraft. I got this idea from C_Corp's PR where he suggested all the craft data to be able to be mapped out in a hashmap. This feature is a monumental task and I believe this will require us to have separate branches in this repository. Another thing to point out is I'm not sure if this system will slow down movecraft as this might increase memory consumption by the plugin.

@TylerS1066
Copy link
Contributor

I have also identified the addition of a speed API as a big "wish", but you bring up a good point with percentages. Ideally, I'd like to have a cache of the block make-up of crafts, which would allow add-ons to read it and handle sinking (or other behavior) in their own way if desired.

You actually touched on a desire of mine as well, similar to how the CraftType rewrite was utilized to refactor all the internal craft flags to use the same system added for external plugins, we should also use the DataTags system for all internal variables. You mention a performance impact, but the reason for using a NamespacedKey based map is that the performance impact of a look up is normalized to constant time. While there will be some minimal overhead, I'm pretty sure the benefits will outweigh it.

A long term goal of mine is to refactor Movecraft itself similar to how the Movecraft-Combat was rewritten. If we develop enough of these APIs for internal usage, we can move each functionality into it's own contained class file. This will not only drastically help bringing in new devs, it also will heavily reduce the maintenance of current functionality.

@goodroach
Copy link
Contributor Author

If I were to make this, I will make it so all the craft's type data will be transferred over to the craft file using NamespacedKey in addition to anything BaseCraft or any other of its implementations may have stored in it. Are there any specific parameters that you want to leave untouched? Or would you like for me to develop a prototype already and see if you like my changes?

@TylerS1066
Copy link
Contributor

With #669 merged, crafts now can have dynamic data. Refactoring speed is another issue logged in #504 and #504

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants