Unnecessary doors and windows are stored in tile_store

[Znote]

Before creating an issue, please ensure:

• This is a bug in the software that resides in this repository, and not a
  support matter (use https://otland.net/forums/support.16/ for support)
• This issue is reproducible without changes to the C++ code in this repository

Steps to reproduce (include any configuration/script required to reproduce)

I will use the house east for Rhyves depot as an example.

1. Start fresh server and go to the house east for Rhyves depot
   

2. Shut down and save house. You will see even though all houses are empty, the tile_store database table will be populated with garbage. <-- Even when there is no modifications to these houses in-game like a toggled door/window.

3. Open the map in RME and change the wall, door, lamp types of the house:
   

4. Save map modification, start server and go back to the house:
   

Expected behaviour

Door and window should be updated to the new wall type, especially if their open/close state hasn't been altered in-game.

Actual behaviour

The old itemids from tile_store will override the map changes.
This is creating tons of headache for people who wants to update house designs or fix broken houses.

For an example, if the house door was previously incorrectly set to id 6249 (closed door), you wont be able to open it and get the message It is locked.
When you enter RME and update the door to the correct version, start server, it will still load this door, and you don't understand wtf is going on, and just before comitting seppoku realize you need to truncate a table (/remove row for this house id in the database) to fix this. (which might be a bad idea to do in production).

Environment

any, default map followed by a modification to the default map

Reproduction environment

default server and source, RME

Suggested changes:

Before storing doors and windows to stile_store (to preserve open/close state across reboots), compare the item to its source id, if it matches, don't store it in tile_store as it is completely unnecessary.

Sure if the door is closed in RME but house owner has opened it in-game, we might want to save this difference to retain the open door, but otherwise it should not be saved. Having to purge records from a database to fix your map is not intuitive and should not be a part of the development and review process.

[soul4soul]

When cipbia updates their houses they tend to send house owner items back to the depot and in more extreme cases they clear the house owner. Sending items backs to depot is usually for smaller bug fixes such as changing wall types, fixing ladders, fixing tiles belonging to the house. Clearing the owner is for larger updates for example when they merge house together. In both cases these are good ways to handle house updates on a production server.

I'm not sure I understand how your suggestion would fix the current behavior. What happens in the following scenario?

1. A house exists with a closed window
2. A player buys the house
3. The player opens the window
4. The map is updated and the window is replaced with a wall
5. On next reboot the window would be restored in place of the wall?
6. Player closes the window,
7. On next reboot the closed window would still be restored in place of the wall?

[Znote]

When cipbia updates their houses they tend to send house owner items back to the depot and in more extreme cases they clear the house owner. Sending items backs to depot is usually for smaller bug fixes such as changing wall types, fixing ladders, fixing tiles belonging to the house. Clearing the owner is for larger updates for example when they merge house together. In both cases these are good ways to handle house updates on a production server.

I'm not sure I understand how your suggestion would fix the current behavior. What happens in the following scenario?

1. A house exists with a closed window
2. A player buys the house
3. The player opens the window
4. The map is updated and the window is replaced with a wall
5. On next reboot the window would be restored in place of the wall?
6. Player closes the window,
7. On next reboot the closed window would still be restored in place of the wall?

Your right, this would end up as a dangling window.
I was trying to avoid a proposed change where we need to keep track of the relationship between item ids, but that might be necessary. So we might need to group together (open door, closed door) into one state that is compared against the source.

[soul4soul]

How about adding a "last_updated" attribute to the house.xml file and a last_saved column to the appropriate house sql table. The last_updated attribute of house.xml can be written on a save in RME anytime house data is changed. Equally on TFS side if that value is newer then the last saved value the owners items will automatically be sent to the depot on server start up and the tiles_store table can be cleared.

[Zbizu]

I didn't read the source code behind it, but the system seems to replace the default doors correctly when player leaves them open. A mess begins when the door id gets changed. Perhaps not detecting original doors could trigger purging the remembered door data and loading default instead.

First thing to change here should be disallowing stacking one door/window on another when applying data from tile_store on the map. Second thing should be not drawing door/window/bed/lamp when an item of similar attributes isn't on the same tile in the map file (eg not load the door when entire wall was moved).

While we are at it, the wall lamps and beds could also be looked into. Last time I remember wall lamps state wasn't even saving.

[gesior]

How about adding a "last_updated" attribute to the house.xml file and a last_saved column to the appropriate house sql table. The last_updated attribute of house.xml can be written on a save in RME anytime house data is changed. Equally on TFS side if that value is newer then the last saved value the owners items will automatically be sent to the depot on server start up and the tiles_store table can be cleared.

Tracking changes in 2 systems (.xml and database) will fail in some scenario. Example: remove wall in house, save map, add same wall in house, save map. RME detect changes each save and lose information 'what was before'. It would mark house as updated, even if it's exactly the same as it was.

It would be easier to add function that generate some 'house design hash' on server start based on .otbm. If house hash is different than last saved in database, it would move items to depot of house owner and reset door access lists.

[Kamenuvol]

In vanilla they save it directly on the map.
For here, I would check the item id and its transform on use. If both are different from the map then substitute. but it will reset the state (on/off, open/closed)

[gesior]

In vanilla they save it directly on the map. For here, I would check the item id and its transform on use. If both are different from the map then substitute. but it will reset the state (on/off, open/closed)

It won't fix all issues. Example:

• player put tapestry on wall
• owner put doors in place of that wall

Only fix for this is to move all items to depot, if 'map design' changed.

'Map design hash' algorithm for single house:

• get all tiles
• from each tile get all items and add them to string in format "x,y,z,itemid"
• generate SHA1 from that string

House design changes are not a thing you do often. It should be handled properly (apply changes on server after restart), but we don't need to write 1000 lines algorithm to try to prevent moving items to depot.

[Zbizu]

map is being loaded after script systems so technically we can use doors.lua to assist the detection or even code a special method that would send "door transform map" back to place available from c++ side

something like local door = Door(itemId); door:setTransformId(nextId); door:register()