Add repair button

[oversword]

Since #45 we no longer go through the old update_formspec logic which would automatically repair stations on punch.

I have also recently found all the travelnets I configured missing from their networks - which is exactly what this repair behaviour is intended to fix.

To re-allow the repair behaviour, I have added a button to the top right of the primary formspec that allows you to run the repair logic.

The same thing cannot be achieved by editing and re-saving the travelnet data because it does nothing if no data has changed, the repair logic is required.

The downside to this is that the form is getting a little cramped, I'm going to bring this up in an issue elsewhere #53

[OgelGames]

This seems like the worst way to handle this, why not just keep the original method of repairing it? (punching)

Also, what is this code for?

travelnet/update_formspec.lua

Lines 23 to 28 in 9ea2c0d

	local success, result = travelnet.actions.repair_station({
	pos = pos,
	node = node,
	meta = meta,
	props = props,
	}, {}, minetest.get_player_by_name(puncher_name))

[S-S-X]

This really should not require any workaround buttons on formspec.

Somewhat repeating what @OgelGames talked about:
If new method for formspecs is not yet able to handle all situations then restore that automated network update and fix root cause later.

[oversword]

This seems like the worst way to handle this, why not just keep the original method of repairing it? (punching)

The punch and rightclick behaviours have been unified, so now it works the same way on any interaction. The whole point of those changes was to remove the need to punch it to update it. I also don't think it's a very intuitive way of interacting with the travelnet, why would punching it fix it?

Also, what is this code for?

This code is to preserve the behaviour of update_formspec as much as possible for those still using it, we no longer use it in the actual code however, because it doesn't make sense anymore, there is no metadata formspec to update

[oversword]

This really should not require any workaround buttons on formspec.

I really don't see it as a workaround

If new method for formspecs is not yet able to handle all situations then restore that automated network update and fix root cause later.

It's not automated, it still requires user interaction. This just makes it one extra click, makes it obvious what it's doing, and doesn't do it unnecessarily every time you interact with the travelnet via punch. The new method of formspecs is designed to handle exactly this kind of thing, that's why only a few lines need to be added to make this work.

[S-S-X]

Having this button wont practically affect performance at all, it does not really make any significant performance improvements as everything needed is already loaded.

If formspec is broken after station is added then fix adding stations instead of adding button to repair broken code, if mod code is not working and button is needed to fix it then it very much it just dirty workaround for broken code.

While punching was required it did also open formspec, now opening formspec wont anymore execute workarounds to fix broken stuff. Instead of requiring button to execute that workaround just do it when opening formspec.

why would punching it fix it?

Nobody is asking for that behavior here, it should be fixed with any interaction that displays travelnet form.

It's not automated, it still requires user interaction. This just makes it one extra click,

Instead of requiring that one extra click make it require zero extra like how it was before.

doesn't do it unnecessarily every time you interact with the travelnet via punch.

This is completely insignificant, you're talking about completely unnecessary micro optimizations here, this only happens on user interaction not in any long running loop or every globalstep. AFAIK it does not need to load anything as all needed data should already be in memory.

[oversword]

If formspec is broken after station is added then fix adding stations instead of adding button to repair broken code, if mod code is not working and button is needed to fix it then it very much it just dirty workaround for broken code.

This isn't caused by the code, it's caused by data loss, that can happen any number of ways.

Nobody is asking for that behavior here, it should be fixed with any interaction that displays travelnet form.
Instead of requiring that one extra click make it require zero extra like how it was before.

That's not how it worked before, you would need to punch it to run this repair logic

Having this button wont practically affect performance at all, it does not really make any significant performance improvements as everything needed is already loaded.
This is completely insignificant, you're talking about completely unnecessary micro optimizations here, this only happens on user interaction not in any long running loop or every globalstep. AFAIK it does not need to load anything as all needed data should already be in memory.

I agree, but it also affect user experience, how am I supposed to know that punching the travelnet will re-attach it? Also, doing this on every formspec open could cause the travelnet to become inoperable: if you had a network that lost a station somehow, then added more stations to reach the limit, then tried to repair a station while opening the formspec you would instead get an error about the max station count, meaning you could never remove the station because you can't open the formspec to do so.

I can restore the original on-punch behaviour, but I really don't think it makes sense to do this on rightclick too, and I think it makes a lot more sense to have a button that specifically performs the action so you know what you're doing when you do it.

[S-S-X]

This isn't caused by the code, it's caused by data loss, that can happen any number of ways.

See #8 which has discussion about this topic, specifically fixing travelnet database after database is lost.
For some situations one should use that to rebuild database completely, for example after bringing in stuff with we/be/local save or just removing your travelnet db.

For other situations where only few travelnet boxes lost data somehow there's options discussed here:
Add repair button or just make it work without adding any buttons.

That's not how it worked before, you would need to punch it to run this repair logic

Well, I just tested previous version and it does work that way. Punching collects data again, refreshes formspec and then displays formspec. Everything happens with single click.

how am I supposed to know that punching the travelnet will re-attach it?

Why you would need to know that if opening formspec fixes problems just like how it did when opening formspec by punching? I am not suggesting to revert all changes made so far but to simply make it collect data and fix problems whenever formspec is opened.

Also, doing this on every formspec open could cause the travelnet to become inoperable: if you had a network that lost a station somehow, then added more stations to reach the limit, then tried to repair a station while opening the formspec you would instead get an error about the max station count, meaning you could never remove the station because you can't open the formspec to do so.

I'm not sure if I understood that correctly but why would "repair" button be different? Or maybe currently I can ask how it is different?

I can restore the original on-punch behaviour, but I really don't think it makes sense to do this on rightclick too

It makes a lot more sense to have single action to open formspec that simply has correct data instead of having any separate action to fix things especially since there's no significant downsides.

Following is for related future development, how to actually fix it and some other little benefits:

To improve data integrity, if continuing with mixed metadata and dedicated database, things should be fixed and improved by changing how storage works and when it is updated.
For this I would propose removing metadata completely or making it last resort fallback that is written but only used if repair option (exactly #8 or similar) is enabled, if not then use already cached lua tables.

This option also comes with other little benefits (downside is that players cannot download database while saving map):

• Always up to date formspecs (hopefully nobody mentions unloaded blocks / deleting mapblocks from database).
  • Explain just in case: you only need to lookup meta if box is not in db. And you only need to write meta when missing from db skipping all unnecessary metadata access completely (if keeping for backup / player download).
• No need for any repair workarounds.
• Faster compared to c++ calls.
• If all meta is removed also reduces network traffic a little bit which is always good thing.

Until then keep it clean and do not attempt to optimize things that do not need any optimization and do not add "repair" buttons if you do not add some in game mechanism that requires repairing simulated damage (probably should use items to repair in that case if not in creative mode).

[BuckarooBanzay]

@oversword i rewrote the storage mechanism to use the mod-storage:

travelnet/persistence.lua

Lines 36 to 49 in 8d1c55d

	-- returns the player's travelnets
	function travelnet.get_travelnets(playername)
	local json = storage:get_string(playername)
	if not json or json == "" or json == "null" then
	-- default to empty object
	json = "{}"
	end
	return minetest.parse_json(json)
	end
	-- saves the player's modified travelnets
	function travelnet.set_travelnets(playername, travelnets)
	storage:set_string(playername, minetest.write_json(travelnets))
	end

Is this PR still relevant? How are the corruptions even happening in the first place? 🤔