wiremod / advdupe2

Advanced Duplicator 2
http://wiremod.com
Apache License 2.0
89 stars 60 forks source link

Fix copying special "Data" attribute #439

Closed MattJeanes closed 11 months ago

MattJeanes commented 11 months ago

As noted on duplicator.RegisterEntityClass there is a special case "Data" that can be provided to tell it to essentially copy the entire entities data structure and any properties set on it.

This appears to be broken in advdupe2 (working in built-in duplicator) in that while it does handle it on the pasting side, it does not handle it on the copying side properly.

This PR fixes that, and replaces the entire entity copy table with a copy of the entity table, just like it does in reverse on the pasting end, and just like how the built in duplicator does it as well.

I'm not going to say this is definitely the perfect or correct fix and there may well be good reasons as to why it's done the way it is, I'm no expert on this codebase but there is a clear regression with comparison to the built in duplicator here.

I've tested this against an experimental version of my TARDIS addon with duplicator support which is how I found this issue, we're using the special "Data" case because we're going for "best effort" duplication and it seems to work pretty well. If you'd like to test it yourself you can do so on the saves_dupes_fix branch of the addon (if it doesn't exist when you click it it's because it's been merged into dev since).

Thanks!

MattJeanes commented 11 months ago

Wow, thank you for the super quick review - much appreciated!

MattJeanes commented 10 months ago

Hey @thegrb93 - we're planning to release our side of this functionality in around a week and I was just wondering when the workshop version will see an update to include this (and the further fixes mentioned in #441, sorry about that!)

thegrb93 commented 10 months ago

I'll go ahead and do it now

MattJeanes commented 10 months ago

Legend, thank you!