place_schematic places old(outdated) schematic even if schematic file got updated

[adrido]

minetest.create_schematic updates the old schematic file, but minetest.place_schematic places the old shematic

Reproducible with stable Minetest 0.4.14 on Windows 7 (single player) and on Debian (server)

Easiest way to reproduce is using worldedit, but each other mod that uses create- and place_schematic works.

1. Select a worldedit area
2. Execute //mtschemcreate test
3. Execute //mtschemplace test --> Places the schematic test as created in step 2.
4. Modify the nodes inside that area
5. Execute //mtschemcreate test again
6. Execute //mtschemplace test again
   • Expected behaviour: Places the schematic test as created in step 5.
   • Real behaviour: Places the schematic test as created in step 2.

Note: It places the schematic file created in step 5 if you don't execute step 3.

from debug.txt:

2016-06-19 17:01:47: ACTION[Server]: create_schematic: saved schematic file '..\worlds\test/schems/test.mts'.
2016-06-19 17:09:22: INFO[Server]: ObjDefManager: added schematic: name="..\worlds\test/schems/test.mts" index=4 uid=61
2016-06-19 17:14:44: ACTION[Server]: create_schematic: saved schematic file '..\worlds\test/schems/test.mts'.

[kwolekr]

Is this behavior unexpected...? I think it's pretty clear from the documentation that this is supposed to happen.

[paramat]

kwolekr coded the schematic code. If it is a bug possibly a world edit problem?

[adrido]

If I call place_schematic filename, I would expect that it places that schematic saved in filename and not something old.

@kwolekr I cant find documentation about this behaviour. It is not noted in lua_api.txt:

• minetest.place_schematic(pos, schematic, rotation, replacements, force_placement)
  • Place the schematic specified by schematic (see: Schematic specifier) at pos.
  • rotation can equal "0", "90", "180", "270", or "random".
  • If the rotation parameter is omitted, the schematic is not rotated.
  • replacements = {["old_name"] = "convert_to", ...}
  • force_placement is a boolean indicating whether nodes other than air and ignore are replaced by the schematic
• Returns nil if the schematic could not be loaded.

Schematic specifier

A schematic specifier identifies a schematic by either a filename to a Minetest Schematic file (.mts) or through raw data supplied through Lua, in the form of a table. This table specifies the following fields:

• The size field is a 3D vector containing the dimensions of the provided schematic. (required)
• The yslice_prob field is a table of {ypos, prob} which sets the yposth vertical slice of the schematic to have a prob / 256 * 100 chance of occuring. (default: 255)
• The data field is a flat table of MapNode tables making up the schematic, in the order of [z [y [x]]]. (required) Each MapNode table contains:
  • name: the name of the map node to place (required)
  • prob (alias param1): the probability of this node being placed (default: 255)
  • param2: the raw param2 value of the node being placed onto the map (default: 0)
  • force_place: boolean representing if the node should forcibly overwrite any previous contents (default: false)

About probability values:

• A probability value of 0 or 1 means that node will never appear (0% chance).
• A probability value of 254 or 255 means the node will always appear (100% chance).
• If the probability value p is greater than 1, then there is a (p / 256 * 100) percent chance that node will appear when the schematic is placed on the map.

[sofar]

@adrido but the API for minetest.place_schematic() does not involve filenames, so you may be seeing a bug in worldedit, which is the mod that provides the //mtschem(create|place) commands. @jeija may be able to confirm or not.

[adrido]

It is not related to worldedit. I used the Worldedit commands only as example, how to reproduce this behaviour. The bug occur also in my Gates mod, and makes it nearly unusable. EDIT: Every mod that uses create_schematic and place schematic is affected by this.

minetest.place_schematic(pos, schematic, rotation, replacements, force_placement) Place the schematic specified by schematic (see: Schematic specifier) at pos

and

A schematic specifier identifies a schematic by either a filename to a Minetest Schematic file (.mts) or through raw data supplied through Lua, in the form of a table.

so param 2 schematic can be either a filename or a table. In my case its always a filename.

At least I would expect the same behaviour as in Minetest 0.4.13 where it worked correct.

[paramat]

Could it be that when 'test' is saved the second time it is a duplicate so gets a filename like 'test.2' or something, then placing 'test' grabs the first version?

[sofar]

Relevant:

https://github.com/minetest/minetest/blob/master/src/script/lua_api/l_mapgen.cpp#L1282

This code is optimized to assume that schematics won't change, so that it doesn't need to re-read the schematic from file thousands of times, which would kill mapgen placement of schematics.

Changing schematics is fine, just don't save them to a file, instead, maintain them in a lua table. I think that should just work - reading the code it appears that if you pass a lua table, it will use the contents of the table each time.

[sofar]

My current assessment that this is expected behavior, and therefore not a bug. The documentation could be updated to reflect this behavior.

If you want to change schematics at run time, you can read the schematic manually from file yourself, and pass the schematic as table to place_schematic(), each time modifying it as needed. This also prevents needless saving/loading.

[est31]

@kwolekr I couldn't find a place where the doc says this. Can you point to it?

[est31]

Its bad if somebody can't reload a schematic from a file, just because earlier in the execution that schematic has already been loaded once.

Loading to a table is not a good solution as it is slower.

There is lots of abstractions going on with ids being returned that can be re-used.

So either we let schematic code reload the file each time it is used as file, and ask mod developers to use schematic ids to speed up, or we add special schematic specifiers e.g. of the form {filename = "schematic.mts", reload = true}.

I do not think that its a good idea to have inconsistencies in API behaviour where different functions reload schematic files and others use the pre-cached value. Its against this "schematic specifier" abstraction idea by making things more complicated.

[adrido]

If the schematic is cached, then there is already a map filename --> cache, right? Than a probably simple solution could be if you call create_schematic(p1,p2,filename,..) check if the file is already cached and if yes, update cache and save to file, and if not, just save to file. This would fix this issue and would not require to change the API. Also it would be pretty fast, because it can still use the cache. But this would not help if you update the schematic file externally.

[Wuzzy2]

Hi. I understand the rationale of requiring the mapgen to cache the file so the identical schematic is not loaded 1,000,000 times. This just makes sense.

Still, there is an use case for loading the schematic again from the same file. The use case are schematic editing tools so there's a quick way to load a schematic and test a newly created schematic. Having a high-quality schematic editing tool is my goal and it will speed up subgame development considerably.

I like the previous suggestion with simply adding an optional reload parameter. I don't care about the default value. Adding this would be a very good solution IMO.

[paramat]

Unfortunately beyond my ability to code this, anyone interested?

[Sokomine]

The replacements parameter is also cached. It is not possible to place the same schematic twice with diffrent replacements without shutting the world down and starting anew.

Caching is reasonable for schematics used as decoration for mapgens. The trouble is: The behaviour is not documented. The main documentation, lua_api.txt, needs to be updated now to reflect how the api function works - not when this problem as such has been solved finally in the future.

Instead of an additional parameter to force-reload the schematic without cache, it would also be possible to offer a function - i.e. minetest.discard_schematic( filename ) or minetest.forget_schematic( filename ) or something like that - that just removes the cached data for that particular filename. Maybe that is easier to implement.

[paramat]

PR #6889 merged. Docs now describe the behaviour.

[paramat]

Now documented so any core dev support for the suggestions?

[SmallJoker]

Documentation does not solve the problem that the cached schematic cannot be reset in Lua.

[paramat]

Yeah i know :) If you support the suggested feature do remove 'possible close'.

[MirceaKitsune]

I hope this limitation can be solved and a way to reset the cached schematic implemented. This will be problematic for my Structures mod, which relies on the ability to design buildings then import / export them freely each to their own file. If it isn't I'll have to attach date strings to each schematic's file name which will be a lot more messy in my opinion.

In the meantime it looks like this might have broken importing a schematic more than once at all: As described in #8106 I can only call minetest.place_schematic once per session, after that I get a "failed to get schematic" error.