luk3yx / minetest-flow

Yet another formspec library for ~~Minetest~~ Luanti
GNU Lesser General Public License v2.1
7 stars 2 forks source link

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

Open Lazerbeak12345 opened 1 year ago

Lazerbeak12345 commented 1 year ago

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 commented 1 year ago

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

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

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 commented 5 months ago

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

Lazerbeak12345 commented 5 months ago

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