entuland / tpad

A teleporter-pads mod for Minetest
MIT License
5 stars 1 forks source link

Owner-less tpads cause errors #5

Closed BramvdnHeuvel closed 7 months ago

BramvdnHeuvel commented 7 months ago

Tpads without an owner cause the global network to break. When an owner-less tpad is broken, they add a field to the mod storage that cannot be parsed properly.

How to replicate

  1. Start a MineClone2 world.
  2. Load the tpad mod and another mod that generates the tpad naturally. See this bug report.
  3. Break one of the naturally generated tpads.
  4. Place the tpad (or access any other one), and press the "Global network" button.
  5. The game crashes with a tpad error.

What is going wrong?

The issue is related to the tpad mod being unable to handle tpads that have no owner. Usually, the tpad storage in mod_storage.sqlite looks as follows:

modname|key|value
tpad|max_global_pads_per_player|4
tpad|max_total_pads_per_player|100
tpad|pads:Tony|return {["(214,13,-892)"]={type=1,name="Stark Tower"}}
tpad|pads:Natasha|return {}
tpad|pads:Nick|return {["(300,-8,20)"]={type=4,name="S.H.I.E.L.D. base"}}

However, when destroying a tpad with no owner, the mod storage is updated to the following:


modname|key|value
tpad|max_global_pads_per_player|4
tpad|max_total_pads_per_player|100
tpad|pads:|return {}
tpad|pads:Tony|return {["(214,13,-892)"]={type=1,name="Stark Tower"}}
tpad|pads:Natasha|return {}
tpad|pads:Nick|return {["(300,-8,20)"]={type=4,name="S.H.I.E.L.D. base"}}

A new field is added where the key is simply pads: with no owner attached. When looking up the global network, this makes the tpad mod crash.

entuland commented 7 months ago

Thank you @BramvdnHeuvel - could you check the fix in the latest commit and let me know if that seems good for now?

https://github.com/entuland/tpad/commit/61d4af7fa262132329590649c1212d865ef4aa33

Not sure I'd be able to investigate about supporting procedurally spawned tpads as it happens with the other issue you linked, I would appreciate pointers about what's needed - if it's something trivial to implement I'd be happy to.

BramvdnHeuvel commented 7 months ago

The code looks good - I will test whether the code works in practice! 🙌

Concerning naturally spawning tpads, I have found that the bug is unrelated to the tpad mod - so there's no need to spend time on that.

Since I'm personally not interested in procedurally spawning tpads (I want them to be rare and difficult to craft, not easily available), I would like to run some code (which I'm totally fine with running in a separate mod) that automatically destroys tpads with no owner. That way, even if a mod wrongly generates a tpad, I can stop players from finding them.

entuland commented 7 months ago

Nice - I saw the comments on that other issue, definitely looks like a caching issue of some sort independent from this mod - by the way, happy to see tpad being used around, in particular if it's being treated as a precious rarity 😄

BramvdnHeuvel commented 7 months ago

I have tested and the code works beautifully. Thanks a lot! ❤️ I'll close the issue.

If it pleases you, I can tell you that it is used as the ultimate item in our personal server. There's a custom block for every player on the server, and players have to find/craft all them to combine them into one tpad. :)

I'm going to write a little script that automatically removes all ownerless tpads (probably with an LBM). Is there a simple way to check who's the owner of a tpad that can be used outside of this mod?

entuland commented 7 months ago

You're welcome - very glad to read about that!

As for checking the ownerless entries honestly not sure - as far as my mod is concerned, the only data it sees is really that file you mentioned, it doesn't even do anything with the dug blocks other than checking whether they can be dug or not:

https://github.com/entuland/tpad/blob/master/init.lua#L493-L503

In theory you could edit that code so that it returns false when ownername is nil or empty, instead of just allowing the block to be dug - then your players wouldn't be able to dig it, but would still be able to interact with it possibly? reading this other section it would seem so: https://github.com/entuland/tpad/blob/master/init.lua#L461-L466

Going through spawned nodes as chunks get emerged or loaded to ensure such malformed instances are automatically removed would be a completely different thing (and honestly I wouldn't know where to start, other than possibly investigating if you can override the actual digging action to run your own checks: then you could probably make it so that the block does get removed from the world but the player doesn't get the item in their inventory)

entuland commented 7 months ago

Oh well, I kind of failed to get your point - if you know how to go about inspecting nodes and removing them, all you need really is checking if the node is a tpad and query the meta to see if it has an owner, just like the first code I mentioned does when checking whether the node can be dug.