SEServerExtender / EssentialsPlugin

Dedicated Essentials Plugin
GNU Lesser General Public License v3.0
18 stars 18 forks source link

Potentially problomatic practice/behaviour of Dynamic Concealment and grid-refreshes regarding Entity Ids #40

Closed DigTron closed 8 years ago

DigTron commented 9 years ago

The Dynamic Conceal and "/utility grids refresh" seems to be causing the entity id to change every time its concealed or revealed (grids refresh conceals and then reveals nearby grids, or it at least appears that way). This is most likely expected behaviour but I felt it was worth noting.

I think it may be an issue because it is a unique identifier for targeting entities, I understand that even Keen have these change often when grids are damaged/merged/unmerged, but even then one id remains persistent. These ids being in a constant state of flux or even changeable on a players whim may contribute to/be causing issues like #22 and more that haven't been highlighted yet. And also explains behaviour seen in my account of #36, meaning that dynamic conceal + protected entities wont work unless the protected grid is exempt from the dynamic conceal AND never gets refreshed.

This affects any plugins that will rely on targeting or exempting based on these id's and also Protected entities. We could put unique identifiers into grid names as a work around but those names don't always persist through merges...I think it is very important that these ids don't change every time the wind blows.

I know your (dodex) looking at doing a big overhaul now that you have SE source to work off of so I'm not expecting much beyond discussion here.

Is it feasible to retain the original entity or even record its id and force the application of it on the new entity? I'm still digesting SESE and Essentials code so I may come back with a solution.

Tyrsis commented 9 years ago

/utility grids refresh was not supposed to ever be used on servers that did not have world join replacement. (Only a few were allowed to use it, and I added this command due to the issues involved with it).

That being said, in order to conceal a ship must be removed from the world. You may not re-add a ship with the same entityId (if you try, it'll be renumbered. It used to just straight up crash before Keen modified it).

Therefore this is not feasible, and IDs can not be viewed as constant. You must use other ways in order to hash (Position + displayname + ownerID can work, though Vector3D + displayname is usually enough).

I never finished protected entities because it just plain didn't work. The main reason is it require source mods to allow true entity protection. All it really did was fix blocks that were broken, but could never truly stop something from being destroyed with enough effort.

You can look at "overhauling" as much as you want, but you can never remove an entity and add it back with the same entityId. You also can not just merely flip the InScene flag, as it doesn't remove the ship from the world (it still get rendered). Dynamic concealment took a very long time to get working and I ran into many many issues. Saying that it can be done better without going through the journey I had to go through to get it to work will end up causing you a lot of the same pain I went through.

A lot of what is in concealment is there for a reason because of issues I ran into trying to get it to work.

dodexahedron commented 9 years ago

I was considering changing concealment to actually just save out the ships being concealed to xml and then loading them back in on reveal. The majority of the logic would remain the same - just the method to persist the concealed ships would be different.