RedMew icon indicating copy to clipboard operation
RedMew copied to clipboard

Thoughts on antigrief.lua

Open grilledham opened this issue 7 years ago • 2 comments

I like this feature, but I have a few concerns about how it works and some ideas for things to change:

  • When undoing the actions of a player I'm not sure if setting any created entities' last user back to the original last user is a good idea. It might just make more sense to set the last user to the admin who ran the command. In effect I think the admin who runs the command is responsible that any new entities that are created are correct. Or if they create future problems such as belts merging in undesirable ways, it's probably better to have the admin's name on that entity as no one is going to accuses the admin of griefing.
  • I'm still worried about entities on the antigrief surface interacting with each other or consuming ups. Maybe we should at least discuss the pros and cons of using ghost entities on the antigrief surface
    • Pros
      • Less ups
      • Entities don't interact with each other
      • Don't block players (we could turn the player into spectate mode when they go to the antigrief surface)
    • Cons
      • Harder to see
      • Harder to implement the code?
      • Can't store items in ghost containers
  • We might want to consider making one antigrief surface per real surface. If the antigrief module ran after map_layout then it could look through all the game.surfaces and create an antigrief surface for each. (Current;y map_layout needs to run after custom commands, because some map modules add commands.)
  • I would prefer it if modules could be easily turned off in control.lua. Sometime people ask me to make a custom version of a map, either for single player or their own multiplayer session. Some of the modules aren't desirable in those cases
  • I would like to make a camera view into the antigrief surface. In my opinion the antigrief module could double as a history tracker of what the factory used to look like, in that case I think we should also track entities that admins place and remove. This is a lot to think about, but I think it's worth discussing.

grilledham avatar Jun 12 '18 15:06 grilledham

  • Last user

it's probably better to have the admin's name on that entity

I agree. I had this discussion internally and decided to use the original persons last user. Both have their pros and cons. I don't think that griefing accusations would be a big issue. But it would make it more consistant and intuitive if the admins name is on the entity. (If you cancel a deconstruction your name will be the last user). Also it will make the code easier.

  • Ghost entities

Less UPS Entities don't interact with each other

I don't think this will ever actually happen. All entities will be unpowered. Even if some odd solar panels or the odd steam engine with a coal buffer may run. I think thats gonna be the exception. Therefore UPS won't be an issue.

Don't block players

I don't think thats a problem. But yes we can make them go into spectate mode.

Another pro would be: Less script updates. I think we should keep script updates down and the global table down wherever we can. I don't think if it actually matters or if its just my preference.

I agree with your cons. I think they outweigh the pros.

  • Different surfaces

We might want to consider making one antigrief surface per real surface

I actually consideres this, most parts of the code should support multiple surfaces. Some parts don't that would be rewritten. I just didn't do it to keep things simple as a proof of concept. I have nothing against multiple surface support. Although it may be a low priority.

  • Disable in control

I would prefer it if modules could be easily turned off in control.lua

I agree. I just found for custom commands to own it more object oriented. Maybe we can find a middle way? Maybe we call a function from control.lua that disables/enables antigrief?

  • Camera

I would like to make a camera view into the antigrief surface

That would be helpful. But mostly for actually finding the griefer. I don't know if we should allow players to view the ag_surface(s). Mostly because i don't want the griefers to know the tools we have. Makes them griefe more creatively. I also don't want to get to far away from vanilla as to not scare people away. But thats just my opinion.

PS:

Current;y map_layout needs to run after custom commands, because some map modules add commands.

We should fix that. Maybe put the code that hijacks commands.add_command into utils?

Valansch avatar Jun 13 '18 13:06 Valansch

Ghost entities / Performance

I think we need more data to make a meaningful decision here. I ran an experiment where I load a save, set game speed to 100, go into map view and zoom out fully. Then record what I perceive to be the average stable ups I get. Then delete the antigrief surface and record the new ups. For the final save for the beach map I went from 228 to 330, for the factory map a save from this morning I go from 530 to 620. It would probably be good if you performed similar tests so we can compare results.

It's also worth nothing that I have no evidence ghost entities would perform better, I just assumed they would. It can be surprising which entities cause problems and which don't. I ran another experiment where I generated a void map with nothing but small power poles every 8th tile to prevent them from connecting. With 1600 poles It took 6ms to process the Electric network. Connect all the poles and it takes less than 0.1ms.

Disable in control / custom commands

I'm not sure what to do here we have opposite opinions on what to do with custom_commands.lua. But for now we could solve the problem with an init function in antigrief.lua that registers the events, and just call it in control.

I agree that we should move the commands.add_command function out of custom_command. Maybe we make a init.lua for stuff that has to run before any other module. But I can't think of any other code that would go in there yet.

Camera

I agree that letting the griefers know about the tools we use doesn't seem wise. But I like trusting people, so I would vote for letting regulars view the camera. I think if we give regulars more powers to find griefers that only helps us. Also in my experience it's really easy to tell when someone is a griefer or not normally within less than 30 minutes of play time, so I do trust our regulars.

If we do allow regulars to use the camera, it might be a good idea to raise the on_player_promoted event when a player is promoted to regular, or we add new events something like

defines.events.on_regular_promoted = script.generate_event_name()
defines.events.on_regular_demoted = script.generate_event_name()

So I can add and remove the camera for the player. If we add new events that code would have to run before any other module

Different surfaces

I agree that it's a low priority, I don't have any current plans for multiple surface maps.

grilledham avatar Jun 13 '18 16:06 grilledham