Xandaros / evolve

An advanced admin mod for Garry's Mod
50 stars 39 forks source link

Cleanup Command Not Cleaning Up All Entities #176

Closed bellum128 closed 3 years ago

bellum128 commented 3 years ago

The cleanup command seems to not be removing all entities on my server, and I believe it is because of the following code:

https://github.com/Xandaros/evolve/blob/05a3c4b4e1b29883378d26e146008338d03ddecc/lua/ev_plugins/sh_cleanup.lua#L39-L49 https://github.com/Xandaros/evolve/blob/05a3c4b4e1b29883378d26e146008338d03ddecc/lua/ev_plugins/sh_cleanup.lua#L18

Because my server has permanent prop_physics entities (saved with the PermaProps tool) Because our particular map contains prop_physics entities in the map itself, it appears that class is getting added to this filter list. So my question is, what is the purpose of the above lines of code? It seems as though it was added to maybe prevent removing entities that were present on map start? However most addons I know hook onto map cleanup commands and re-spawn themselves, so I don't believe it is needed if that is the case. This bug would also theoretically be introduced on certain maps that have prop_physics entities built in.

If there's no explanation as to why this code should be kept, I will remove it to fix this bug.

Xandaros commented 3 years ago

It does indeed look like it is meant to prevent cleanup of entities present on map load.

Perhaps instead of storing the class, it should store the exact entities present on load and match against those? I'm not certain it is a good idea to just completely remove the check.

bellum128 commented 3 years ago

it should store the exact entities present on load

Hmm I thought of that as well, but I also checked the source code of the actual vanilla cleanup button in the UI, and it doesn't do any kind of filtering like this... https://github.com/Facepunch/garrysmod/blob/669d95b027dc319c88cf6679f202143325eec853/garrysmod/lua/includes/modules/cleanup.lua#L170

From what I can tell, entities baked into the map reset themselves but don't remove themselves as expected when CleanUpMap is called, and at least the PermaProps addon I use for custom props, and I assume any other persistence system, would hook onto the cleanup event. So I'm not quite sure of a specific scenario as to why the filter would be necessary given these two things...is it possible there was a time in which world entities didn't automatically respawn on CleanOnMap, resulting in this code being needed in the past but now being redundant?