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-request/idea : rendering for node meta #11

Open
Lazerbeak12345 opened this issue Oct 21, 2023 · 6 comments
Open

feature-request/idea : rendering for node meta #11

Lazerbeak12345 opened this issue Oct 21, 2023 · 6 comments

Comments

@Lazerbeak12345
Copy link
Contributor

This isn't needed for anything right now [1], but I've been thinking about this:

Do not use this API with node meta formspecs, it can and will break!

It should be quite easy to make a very basic node meta renderer, akin to :set_as_inventory_for. As a security feature, it could simply set player to nil while calling the build function.

That's the most basic level of support, but there are two other things that would be nice, but hard to support:

  1. To determine the formspec version it'd have to track the lowest known formspec version (and which players are on that version), at the least.
  2. The formspec size would also have to be calculated this way, though flow (currently) doesn't do anything with size.

If either of the above are added, flow would have to re-render formspecs that need to be downgraded (or upgraded?) on occasion. My recommended fallback for this scenario is just always to assume the oldest formspec version flow supports, but have that variable overrideable or something so FS51 can make it lower. I don't know what to do about formspec size.

An unanswered question would be this: Would the player need to be passed to action callbacks? It would make sense if so, since the action likely needs to know who clicked it.

[1] It would be useful for mtg forks that use flow whenever possible. As of time of writing, I'm not aware of such a project. It would also be useful if someone, like me, made a mod to provide integration APIs between (my mod) Sway and MTG's chests. It would also be useful for things like flow-rewrites of Sokomine's chest mod, Hopper, or any other interactable voxel using node meta.

@luk3yx
Copy link
Owner

luk3yx commented Oct 21, 2023

There are multiple problems that would have to be worked around (and probably more that I'm not thinking of):

  • Callbacks can't be persisted across server reboots but node meta formspecs are. While you could regenerate the form and then run the callbacks, any automatically named buttons would be broken.
  • You would probably have to remove the callbacks and ctx tables for unloaded mapblocks to free up memory.
  • Fields would have their values overridden if multiple players were using the form at once (though I don't think this is specific to flow).
  • If a mod changes the form, existing nodes won't get updated (this is a problem for node meta formspecs in general, you'd need to register an LBM to update all of the old nodes).
  • Would ctx.form have to be serialised and stored in the node metadata? If it doesn't, then what happens to the values already in fields, checkboxes, etc when the ctx table gets deleted or the server is rebooted? The old values would still be in the formspec but not in the new (empty) ctx table.

but there are two other things that would be nice, but hard to support:

I don't think either of those are possible with node meta formspecs without jankiness (an old client would briefly see a formspec targeting newer clients before the server could respond to the right click event and update the formspec, and attempting to update them before players click on them would probably cause lots of useless form redraws and network traffic).

Are there any advantages to node meta formspecs over calling form:show(clicker, {pos = pos}) from an on_rightclick callback? The only one I can think of is that the client can open the formspec immediately without waiting for a response from the server, however that might be negated if any per-client changes to the form are needed.

If you were wanting to try using flow with node meta formspecs then maybe form:render_to_formspec_string could accept nil as a player argument (and then you'd have to pass one to the event function).

@Lazerbeak12345
Copy link
Contributor Author

All good points. I think everything has been addressed. Perhaps just adding the recommendation to use interaction callbacks instead of node meta would be all that is needed.

@luk3yx
Copy link
Owner

luk3yx commented Oct 22, 2023

If you were wanting to try using flow with node meta formspecs then maybe form:render_to_formspec_string could accept nil as a player argument (and then you'd have to pass one to the event function).

I'd be happy to implement this if you were wanting to try experimenting with node meta formspecs in flow_extras or another mod, though I'm not sure about including a node meta formspec API in flow itself (unless the implementation somehow works around most of the issues and isn't too complex, but some of the issues sound really hard to work around).

@luk3yx
Copy link
Owner

luk3yx commented Nov 3, 2023

I have experimented with this, though I didn't manage to find a solution to the issues I mentioned:

  • If you restart the server any automatically named buttons wouldn't work the first time you pressed them.
  • If you restart the server then it looks like form input is remembered, but if you actually try and do anything it will forget
    • This could be worked around by storing ctx in the node's metadata but then you couldn't store function values in ctx and doing something like minetest.after(3, function() ctx.test = 1 end) would be unreliable.
  • Cleaning up contexts and callbacks for unloaded mapblocks could be done by using a globalstep to check whether the node is still loaded in, though I don't really like this.

I also replicated the API (register_as_node_formspec_handler(form, node_name)) but used form:show and closed the form in the on_destruct callback which seemed to work better, though would mean you'd have to do protection checks in every callback (in case someone else protected the node in the meantime), and I don't think WorldEdit calls on_destruct callbacks so you may also have to check that the node still exists.

@Lazerbeak12345
Copy link
Contributor Author

I think that in this case, a field missing a name should error out if it requires one. Probably an assert.

@Lazerbeak12345
Copy link
Contributor Author

As I mentioned earlier, perhaps the better way is to not set the node fs string, and instead use node interaction callbacks. A wrapper function might still be nice 🤷‍♂️ but idk

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

No branches or pull requests

2 participants