Framed paintings are sometimes popped off the Memorial landmark

[JiFish]

Framed paintings are sometimes popped off the Memorial landmark. This doesn't effect all of them, and other framed paintings (e.g. in studies) seem to work fine.

You can see a broken one in the foreground and a working one in the background of this image:

[sshipway]

The one in the background also seems to be out of position, IE to the right of where it should have been? Both these memorials are facing the same way, which was the thing I first thought might be the issue (if only ones facing E were falling off for example) so its not that. Seems odd that it only pops off some times.

My guess is that the code is generating some invalid tags on the painting that cause it to error and then pop off, since 1.9 and 1.10 are more strict about tag format. I know that the painting.tag.display.Name can get big, maybe that is it? There should also be a sign below the picture with "memorial to " and the picture name, which may now be illegally long.

[JiFish]

Since I haven't the time to look at this, I have temporarily disabled the Memorial landmark. Hopefully, that will avoid some confusion.

Don't forget to re-enable when fixed.

[sshipway]

It seems to be that the problem is linked to specific pictures; some (the cheshire cat) always work, and others (most of the famous paintings, and the chess pieces) pop off. Once picked up in-game and replaced, they are fine thereafter. Presumably there is something converted and altered when they are placed but I cannot yet find out what it is.

---

So further investigation shows the problem is in fact with the sequence. The initial landmark works; then subsequent may work, or be off position by 1 to the right, but after they start going wrong they ALL go wrong. The paintings are created correctly, but the frame entity somehow goes wrong, possibly due to the way I'm calling the function. Maybe there are some static data I am assuming are instantiated.

[sshipway]

Right, I've found a cause. The get_entity_other_tags function appears to return a static object and not a new one, so the multiple calls being made to create all the landmarks are causing the data to be somehow damaged.

To get around this, I've used copy.deepcopy() on the returned object before passing it to addentity; I've also done a copy.deepcopy() on the map object passed as the parameter ItemTags. This has fixed the popoff problem in all cases I've tested so far... however, there is still an apparently random case where the frames are placed one tile to the right on all landmarks after a certain point. I think there is yet another object somewhere in get_entity_other_tags which should be deepcopy'd.

However, I'm not convinced this is the correct solution. Better would be to correct the get_*_tags functions to return a clean fresh object that is independent of anything static or passed as a parameter. I'll investigate this tomorrow, time permitting (family bereavements are sadly preventing me from working on this as much as I would have liked this holiday season)

This may also be the root cause (or a similar cause) of #409 and #429 , though this is just a guess.

[sshipway]

OK, fixed. Will push soon. The problem is down to the get_entity_other_tags not doing a copy.deepcopy() on the Pos parameter before modifying it, and on the ItemTags before using it. These have modifications that then accumulate until it breaks the entity in interesting ways. I've made both deepcopy in the utils.py now rather than in landmarks.py as this will help everywhere. There is a good chance that this has been breaking other things as well elsewhere, and that get_entity_mob_tag should also do some deepcopying before using objects.

[sshipway]

Sorry, work has been too much recently, not to mention 2 family funerals. Apologies for not pushing yet, but I should really do more testing and haven't had the time. It will come.

[sshipway]

This is really driving me up the wall. The previous fix does not seem to have done so after all (though it has reduce the frequency it seems). It is definitely a drift towards -ve Y and +ve X that slowly builds up after mutliple calls (I spawned next to one and saw the frame floating above the wall fro 1sec), but I can't find out where it is yet.

[sshipway]

Fixed in merge #431

Problem was mainly due to the functions using int() rather than math.floor() to convert a float Pos to an integer TileX. This only affects treasure hunts because they can let the (relative) Pos go negative. However the need for a deepcopy of the passed Pos and tileentity before modification was also causing problems which masked the issue.