Facepunch / sbox-issues

175 stars 12 forks source link

Components should call simulate and other entity methods automatically #1351

Closed Jake-NSW closed 9 months ago

Jake-NSW commented 2 years ago

What it is I use components almost everywhere in my code base, but a method I've had to reimplement into all my entity components is simulate.

Would be nice if it was automatically called from Entity itself, instead of me having to iterate over it in 30 different entities myself.

What it should be Adding these to methods to EntityComponent, and them automatically being run from Entity

public virtual void Simulate( Client client ) { }
public virtual void FrameSimulate( Client client ) { }
public virtual void PostCameraSetup( ref CameraSetup camSetup ) { }
public virtual void BuildInput( InputBuilder input ) { }
Jake-NSW commented 1 year ago

Bump 🙂 still see this as a nice thing to have out of the box. Especially with the new Library project types!

Thomas-Lazenby commented 1 year ago

An ideal compromise could be the introduction of an ISimulate interface that's able to be applied to components. Creates a universal contract that determines whether components are eligible to run within our games (We would still have to check components if we should run). Implementing this achieves a balance between providing functionality and avoiding the creation of an excessive number of methods, which could lead to unnecessary overhead.

If you want all components to run, potentially in Game Simulate or Pawn Simulate:

Game.cs:

public override void Simulate( IClient cl )
    {
        foreach(var item in EntityComponent.GetAllOfType<EntityComponent>().Where(x => x is ISimulate).Cast<ISimulate>())
            item?.Simulate( cl );

        base.Simulate( cl );
    }
Weldify commented 1 year ago

I don't see why this should exist. If you need a simulatable component, just make it yourself? With your solution you still have to implement the interface and call it, but it doesnt have enough functionality to be usable - most of the time you want the simulation order to be deterministic so you would also want to include a "simulation priority" value in each component. And then congrats - you now have 2 interfaces that you have to implement.

Overall I dont think its a good idea. If a game wants to support simulatable components properly - it's their job to expose something to make it happen, since things work very differently in all games.

Tl;dr I think this feature is redundant and wouldn't help

Thomas-Lazenby commented 1 year ago

I don't see why this should exist. If you need a simulatable component, just make it yourself? With your solution you still have to implement the interface and call it, but it doesnt have enough functionality to be usable - most of the time you want the simulation order to be deterministic so you would also want to include a "simulation priority" value in each component. And then congrats - you now have 2 interfaces that you have to implement.

Overall I dont think its a good idea. If a game wants to support simulatable components properly - it's their job to expose something to make it happen, since things work very differently in all games.

Tl;dr I think this feature is redundant and wouldn't help

I see your point. The idea of the ISimulate interface isn't to make things complicated. It's really about providing a common agreement on how we handle simulations.

You're totally right about things like simulation order and priority, they're crucial. That's why I suggested that developers implement the interface themselves. It's flexible - it can be tailored to what a game needs.

Yeah, every game is different, I get that. But having this common interface doesn't stop a game from doing its own thing. It just gives everyone a consistent starting point. So in a nutshell, the ISimulate interface is about creating a consistent way of doing things, while still allowing for individual game needs.

Weldify commented 1 year ago

If they still need to add stuff on top, what's the point? If they need to simulate their stuff - it's one line away from being possible.

Also I don't think a common way of doing things is good. You could argue that generic addons could use it, but I don't think thats useful either - generic addons are practically useless for this since they cant access any game functionality.

So it really is redundant. The only way I can see it being useful is making it easier for newbies, but that is easily done with a wiki page

Thomas-Lazenby commented 1 year ago

If they still need to add stuff on top, what's the point? If they need to simulate their stuff - it's one line away from being possible.

Also I don't think a common way of doing things is good. You could argue that generic addons could use it, but I don't think thats useful either - generic addons are practically useless for this since they cant access any game functionality.

So it really is redundant. The only way I can see it being useful is making it easier for newbies, but that is easily done with a wiki page

Clearly, both sides have pros and cons, it's just mainly consistency with my idea so we make it easier for people to agree on an interface so we don't have several interfaces from several libraries needing to implement this.

generic addons are practically useless for this since they cant access any game functionality.

Isn't that the point of an interface?

Weldify commented 1 year ago
  1. People don't have to agree on an interface, everyone does things differently as there is no real reason to
  2. Even if a generic addon can implement this interface, what will it do with? It can't access any game logic eitherway
Jake-NSW commented 1 year ago

An ideal compromise could be the introduction of an ISimulate interface that's able to be applied to components. Creates a universal contract that determines whether components are eligible to run within our games (We would still have to check components if we should run). Implementing this achieves a balance between providing functionality and avoiding the creation of an excessive number of methods, which could lead to unnecessary overhead.

I don't think this is the right way of handling things. You don't get any state and every component is always being predicted, even components on entities where the entity isn't running its simulate loop.

I don't see why this should exist. If you need a simulatable component, just make it yourself? With your solution you still have to implement the interface and call it, but it doesnt have enough functionality to be usable - most of the time you want the simulation order to be deterministic so you would also want to include a "simulation priority" value in each component. And then congrats - you now have 2 interfaces that you have to implement.

What I am asking for is a standardized way of receiving callbacks for certain methods on the entity itself, wither that be on an interface, a virtual function I don't care. As long it exists. The whole point of sbox is to be a playground. I shouldn't have to reimplement the same shit over and over again on each of my entities. Especially when it's something as simple as this.

As for the component order, its defined by how you add your components to an Entity. It should be up to the developer to figure that out. I also think you are doing something wrong with components if you require them to be in a particular order is well. The whole point of components is to decouple logic from the Entity, allowing it to be used on other entity types, without it needing to explicitly ask for other components.

If you do need "order' then you should use some sort of handler component class that gets a other components on the entity that have an interface, or the other components subscribe to an event on the handler.

Also have to ask the question, what are you doing that requires your components to be in a special order?

Even if a generic addon can implement this interface, what will it do with? It can't access any game logic eitherway

What about libraries? Main reason why I bumped this was because of libraries. I want to be able to have a library that can add components to a pawn for example (such as an inventory, or carriable simulator), and the end user / developer should not have to worry about calling a simulate function for components on their pawn as there would be a standardized way to do it already.

Weldify commented 1 year ago

I extensively use components in my game. I use them for movement and weapon abilities. Having them run in a certain order is important for them as some rely on others. Hotloading also has a bug where it sometimes shuffles component order randomly - which will create prediction errors unless you have a deterministic order for them.

As for libraries - I can understand the usefulness of that. It comes down to preference, but for me I would never use something that does things under the hood - I always want to call things myself to know what's going on and to better order it within the loop of my game.

Thomas-Lazenby commented 1 year ago

An ideal compromise could be the introduction of an ISimulate interface that's able to be applied to components. Creates a universal contract that determines whether components are eligible to run within our games (We would still have to check components if we should run). Implementing this achieves a balance between providing functionality and avoiding the creation of an excessive number of methods, which could lead to unnecessary overhead.

I don't think this is the right way of handling things. You don't get any state and every component is always being predicted, even components on entities where the entity isn't running its simulate loop.

Looking further at your counter points below I do agree actually if stuff needs to be manually handled (Except lack of state bit confused on that one) then don't bother using Simulate method if initial idea was suggested, Having ISimulate and IBuiltInput I think wouldn't be a bad idea as it stops unnecessary bloated methods on components. However, I forgot one thing which is consistency of API so my idea is not as ideal compared to this or [Event.Simulate] (lol).

I only put a "quick" thought into the ring but clearly I believe am violating the API design of S&Box with this idea.

Weldify commented 1 year ago

Another thing i want to mention:

the end user / developer should not have to worry about calling a simulate function for components on their pawn as there would be a standardized way to do it already.

The main problem with this is you don't get a say when this runs. Is this going to simulate before the main client simulation or after, how will this affect different games? That's why I want it to remain the way it is now - if you are using a library, you should call into it when you need to - not when the library wants to. Sure, calling simulate automatically will make it easier - but it takes away control

Jake-NSW commented 1 year ago

The main problem with this is you don't get a say when this runs. Is this going to simulate before the main client simulation or after, how will this affect different games? That's why I want it to remain the way it is now - if you are using a library, you should call into it when you need to - not when the library wants to. Sure, calling simulate automatically will make it easier - but it takes away control

Then just have to callbacks? One for OnPostSimulate() and one for OnPreSimulate() or just control the behaviour of where components are simulated using the base.Simulate(). Thats what I currently have on my project and it works pretty well.

I extensively use components in my game. I use them for movement and weapon abilities. Having them run in a certain order is important for them as some rely on others. Hotloading also has a bug where it sometimes shuffles component order randomly - which will create prediction errors unless you have a deterministic order for them.

I also use Components for controlling data on a given entity and use them extensivly. But I don't do it in a way where it does its math one by one, I use the default base data then make all the components iterate over that, then just += the data together all at once. That way the component order doesn't matter.

Looking further at your counter points below I do agree actually if stuff needs to be manually handled (Except lack of state bit confused on that one)

In your code example that would run the simulate call for every component currently in the game at the same time. What I am asking for is the simulate on a component to only be called if the entities simulate is being called.

On a side note, I don't want to turn this into a silly debate. This is just a feature request after all. Whether or not it gets accepted by Facepunch is up to them to decide.

Weldify commented 1 year ago

Nah man, i just have a different point of view, no ill intent in this discussion. Speaking of callbacks, thats a very good idea. I myself wrote some code today that would benefit from that. Im all in for callbacks to be added in the form that you described

LeQuackers commented 1 year ago

Currently we're unable to make generic ability addons (e.g. "double jump"/"midair jump") that work in the default sandbox gamemode (or other gamemodes for that matter) because Components don't have simulate methods/events. The "loop over the Pawn's components and simulate them" idea works, but it would be really nice to have something that's built-in by default.

If this ever gets added, I'd opt for [Event.PreSimulate]/[Event.PostSimulate] and [Event.PreFrameSimulate]/[Event.PostFrameSimulate] events instead of methods; the other methods have already been implemented using the event system and it would allow someone to check for changes during the simulation, e.g.:

If you wanted to make a "double jump"/"midair jump" EntityComponent addon for the default Sandbox gamemode, you would need to check if the Pawn had a ground entity before the Pawn moves to determine if this was a normal jump or a midair jump; if there was a single public virtual void Simulate( Client client ) method or [Event.Simulate] event, that information would very likely be destroyed by other EntityComponents or the Pawn itself.