Reassure previous nil behaviour for tiles

[Montandalar]

The mod dumpnodes which is included with minetestmapper has broken
with 5.6.0 when run with bike mod, due to changes in 18fbc03 which
defines a tiles table for all nodes by default, where previously a nil
was returned if a nil was given. Restore previous behaviour so mods do
not break.

Summary

• Goal of the PR
  Fix compatibility with mods that expect tiles to be nil if they registered their items with tiles = nil
• How does the PR work?
  It removes the entries from the item defaults that were re-added in 18fbc03
• Does it resolve any reported issue?
  Consider this the issue report as well. I saw fit to write the whole PR rather than worry about reporting the issue first.
• Does this relate to a goal in the roadmap?
  This is a maintenance item that I hope will be in 5.6.1
• If not a bug fix, why is this PR needed? What usecases does it solve?

The long-winded version

The mod dumpnodes which is included with minetestmapper has broken with 5.6.0 when run with bike mod, due to changes in 18fbc03 which defines a tiles table for all nodes by default, where previously a nil was returned if a nil was given.

This problem was hard to trace and I used git bisect to find it. I am worried that it may be the cause of hard-to-understand bugs in other mods and that this may not be the only mod affected. A guarantee that nil in leads to nil out seems sensible to me, especially given that most other table properties do not have default values with the exceptions of wield_scale, post_effect_color and selection_box. Surely the fewer defaults necessary the better as well.

The reasoning of code clean-up of obselete code seems sound at first, but the implementation was wrong. In particular, in 2b500d, celeron55 commented:

Remove tiles and special_tiles from node definition prototype because otherwise the old names can't be used

However, now that those obsolete features are gone, since they were deprecated in fd1135c, (git described as 0.4.dev-20120606-11-gfd1135c7a, i.e. in 2012 BEFORE the release of 0.4), empires have risen and fallen, and mods are much more used to a nil value than an empty table value.

Here is the registration used in bike:

minetest.register_node("bike:hand", {
    description = "",
    -- No interaction on a bike :)
    range = 0,
    on_place = function(itemstack, placer, pointed_thing)
        return ItemStack("bike:hand "..itemstack:get_count())
    end,
    -- Copy default:hand looks so it doesnt look as weird when the hands are switched
    wield_image = minetest.registered_items[""].wield_image,
    wield_scale = minetest.registered_items[""].wield_scale,
    node_placement_prediction = "",
})

To do

This PR is a Work in Progress, needing only a regression test, which I am unsure how to do correctly. It seems possible to register item definitions from C++ inside src/unittest but not to call them with the filler values eventually used in minetest.register_node, and I'm not sure to what extent we are meant to call into the Lua API from C++.

• Tested with dumpnodes
• Regression test

How to test

0. Run Minetest 5.6.0
1. Install dumpnodes from minetestmapper
2. Install bike from Hume2's GitLab
3. Load a world with bike and dumpnodes enabled.
4. Run /dumpnodes
5. Observe the crash occurs inside dumpnodes because it assumes that items without textures with have nil tiles, not an empty table.
6. Run Minetest with this PR's code installed in builtin.
7. Repeat steps 3-4, but with this PR, and notice there is no crash.

[Zughy]

I don't think this should be "fixed", because those values are supposed to be declared by default. dumpnodes should check for if next(tiles) then

Or they could be declared as tiles = nil just for clarity

[Montandalar]

Is your conception that they are supposed to be declare by default based on the ancient version of the code from c55 that you uncommented? Because that's the only supporting evidence I see, whereas I do see almost 10 years of practice since then where nil was how it worked. Yes, the mod could be forced to change. But this is the kind of breakage that is contributing to Minetest's perception of continual breakage with each 5.x point release.

[Zughy]

A lot of fields are nil by default and they're specified in that file, hence my second suggestion. By the way, docs don't say anything about the default value https://github.com/minetest/minetest/blob/master/doc/lua_api.txt#L8016

[Zughy]

Closing since the alternative has got two approvals