abraker95 / tanks

2D arcade top-view shooting game
1 stars 0 forks source link

Entity Component System #38

Open ghost opened 9 years ago

ghost commented 9 years ago

This took a long time but I'm happy with the result. I added some components and systems to test if it works and I managed to get a moving tank.

I created a Readme which contains the change log and some explainations about the entity component system ( at the moment, it's just code samples ).

I hope you like it. The advantage with this system is that you can easily combine functionnalities with components, and creating new functionnalities is just creating a new system.

ghost commented 9 years ago

I see two ideas.

A name component for every entity. A generic factory for every type of object. if I understood you correctly.

The first one can be useful. It's always nice to have names for entity. The danger with that method though is that we can have access to entities from anywhere if the programmer knows the name, which is basically the global variable problem.

For the second one, it's like the entity manager I just pushed with one additionnal layer of abstraction. So I guess the tank constructor is a list of new, and in createObject would call Environment::createEntity(). I think we can do abstraction but in another form. The main problem I see here is when we want to add a component to tank for example, we need to do quite a bit of changes and we lose the feel of adding easily components like a true ECS.

ghost commented 9 years ago

I will implement an event system because the current design is not optimized for not so frequent events like collisions.

If we want to play a sound when a collision is detected, we would have to set a flag when a collision is detected and then in the audio system, iterate over all entities and when the flag is detected, play a sound.

An event system would be more optimised and also more clean, I guess. I still need to think about the exact design.

ghost commented 9 years ago

What is the ComponentBase for, by the way? It's just a destructor.

ghost commented 9 years ago

I'm using the power of polymorphism.

Every Component class is extended from ComponentBase. So in the Envionment class, I can have a vector of components just by declaring std::vector<ComponentBase>. The alternative std::vector<void> is not good because upon the destruction of a component, it is not possible to call the destructor of that component. But with the virtual functions ( the virtual destructor in ComponentBase ), I can easily call the destructor.

That's a very useful trick.

ghost commented 9 years ago

Planning to add a nifty feature to the Entity System: variable type components. Got tired of creating new components to house just one variable. So I came up with a solution...

ghost commented 9 years ago

It's just helping you, not the program nor the design. If you feel it's necessary, you can do it. An alternative would be to regroup several components in one file, I think it will feel less "heavy" to add a component like I did with the events.

ghost commented 9 years ago

Still, my approach is more portable than that. Take a look at it when I push, maybe you can improve the design.

ghost commented 9 years ago

I see what that you want some kind of one-line defintion for components. The stdComponent goes in a good direction but the problem is field and struct naming. With this design, two different components can't have the same field's type.

For example, stdComponent is the expire component but it can also be another component. And hasComponents(stdComponent) is true for all of them, differencianting them becomes impossible.

Grouping in one file is still the best I think.

ghost commented 9 years ago

No, there is a difference between StdComponent and StdComponent, right?

I already used it in the UI System's constructor like so: new StdComponentsf::RectangleShape(new sf::RectangleShape()),

ghost commented 9 years ago

I always forget that < > go away in github messages. I meant:

stdComponent<float> is the expire component for example. And if you want another component with one float it is also stdComponent<float>. But then you can't differenciate them. When hasComponent<float>(i) is called all the stdComponent<float> are true.

ghost commented 9 years ago

True it breaks there, That's where components having names would make much more sense.

ghost commented 9 years ago

I think it would be a good idea if a createEvent and a destroyEvent is automatically emited from createEntity() and destroyEntity().

ghost commented 9 years ago

Wouldn't that just be an extra layer of code?

event -> env -> create ???

I think events should be used to perform explicit operations upon triggers. The systems can't be used with button because an action needs to be set in its parameter to do something, which could be anything. Events are great at managing those actions. The creation of an enitiy can be done within the systems because its a predefined action.

On Tue, Dec 16, 2014 at 3:04 AM, Sherushe notifications@github.com wrote:

I think it would be a good idea if a createEvent and a destroyEvent is automatically emited from createEntity() and destroyEntity().

— Reply to this email directly or view it on GitHub https://github.com/Sherushe/tanks/issues/38#issuecomment-67124811.

ghost commented 9 years ago

Don't mix-up. An event is just information not an action. Emitting a create event in createEntity is just for convenience. The other systems can use this information. I was suggesting this because a lot of ecs with an event system do this.

For example the one in the readme: https://github.com/alecthomas/entityx#builtin-events

ghost commented 9 years ago

Oh ok. If put that way, it looks pretty cool.

On Tue, Dec 16, 2014 at 6:10 AM, Sherushe notifications@github.com wrote:

Don't mix-up. An event is just information not an action. Emitting a create event in createEntity is just for convenience. The other systems can use this information. I was suggesting this because a lot of ecs with an event system do this.

For example the one in the readme: https://github.com/alecthomas/entityx#builtin-events

— Reply to this email directly or view it on GitHub https://github.com/Sherushe/tanks/issues/38#issuecomment-67144251.

ghost commented 9 years ago

I have a problem where an entity that is emitting an event executes before the component receiving the event. The events get cleared before the entity receives the event.

On Tue, Dec 16, 2014 at 6:17 AM, A Braker abraker95@gmail.com wrote:

Oh ok. If put that way, it looks pretty cool.

On Tue, Dec 16, 2014 at 6:10 AM, Sherushe notifications@github.com wrote:

Don't mix-up. An event is just information not an action. Emitting a create event in createEntity is just for convenience. The other systems can use this information. I was suggesting this because a lot of ecs with an event system do this.

For example the one in the readme: https://github.com/alecthomas/entityx#builtin-events

— Reply to this email directly or view it on GitHub https://github.com/Sherushe/tanks/issues/38#issuecomment-67144251.

ghost commented 9 years ago

If possible change the systems' order. If it's not possible I will try to fix that.

ghost commented 9 years ago

I'm trying to implement a flag that tells whether to clear the events or not.

On Wed, Dec 17, 2014 at 1:37 AM, Sherushe notifications@github.com wrote:

If possible change the systems' order. If it's not possible I will try to fix that.

— Reply to this email directly or view it on GitHub https://github.com/Sherushe/tanks/issues/38#issuecomment-67282960.

ghost commented 9 years ago

Just make sure it's easy to use.

ghost commented 9 years ago

For some reason component pointers get reset. I am assigning nullptr at createEntity, and assigning it a constructor pointer if it is nullptr in the system update, but it's always nullptr for some reason. It seems to do with how I am passing down the pointer value. Does this seem correct:

sf::Texture* shaderObj = env->get<StdComponent>()[i].data; shaderObj = new sf::Texture();

This seems to copy it too: auto shaderObj = env->get< StdComponentsf::Texture>();

ghost commented 9 years ago

I don't really understand why you're still using StdComponent.

For the first example, shaderObj gets the pointer to the new texture but not the data member. I'm not sure if this is the question. If you also want data to point at the new allocated texture, do something like: env->get<StdComponent<sf::Texture>>()[i].data = shaderObj;

The second example ( like in the source ) should work.

ghost commented 9 years ago

I don't really understand why you're still using StdComponent.

To avoid making structs/headers/source files when it can be avoided. It supposed to take any existing class/struct and make it compatible with the ECS system.

I got it working yesterday, but I will still examine what your solution.

On Sun, Dec 21, 2014 at 4:30 PM, Sherushe notifications@github.com wrote:

I don't really understand why you're still using StdComponent.

For the first example, shaderObj gets the pointer to the new texture but not the data member. I'm not sure if this is the question. If you also want data to point at the new allocated texture, do something like: env->get<StdComponent>()[i].data = shaderObj;

The second example ( like in the source ) should work.

— Reply to this email directly or view it on GitHub https://github.com/Sherushe/tanks/issues/38#issuecomment-67785555.

ghost commented 9 years ago

To avoid making structs/headers/source files when it can be avoided. It supposed to take any existing class/struct and make it compatible with the ECS system

You need to think more about the long-term. It will become more problematic because the components have no names. But I agree that it is difficult to properly separate data into components.

Shaders for example, you can make it much more flexible. Every entity could have a shader component attached to it.

But for the shader like the menu one, it should be a built-in shader in the rendering system, in my opinion. The shader itself is not really an entity. We could do something like where the rendering system has three built-in shaders:

And set these shaders from outside.

ghost commented 9 years ago

It will become more problematic because the components have no names.

Refactor every component to be independent and use it with StdComponent? I was thinking of doing such for the following components, actually: BoundingBox, Solid, Sprite, Texture, Tilemap, and VertexArray. Those components have a single or no variables, and that variable could be in StdComponent. Everything else can be redirected through StdComponent for consistency, or extended from Component if you insist. I know it's going a bit from the ECS approach, but cutting down on resources used is one of the things I focus on. As I said before, ECS approach is really nice externally, but internally it's a mess. This way everything else is more modulated, meaning that we can easily make any class, struct, variable type, a Component using StdComponent if needed to. The following macro is what will make using StdComponent much easier:

The usage would be then something like so:

main_env.createEntity ( "ESC UI", Component(bool, new bool(true), "visible"), Component(sf::Texture, nullptr, "shader_filter"), Component(sf::Shader, nullptr, "blur_shader") );

Tell me what would break, so I can rethink the design. I think this would be a better approach.

Shaders for example, you can make it much more flexible. Every entity could have a shader component attached to it.

I was thinking to use shaders only where needed. Why make extra unused variables to take up memory space?

On Mon, Dec 22, 2014 at 3:04 AM, Sherushe notifications@github.com wrote:

To avoid making structs/headers/source files when it can be avoided. It supposed to take any existing class/struct and make it compatible with the ECS system

You need to think more about the long-term. It will become more problematic because the components have no names. But I agree that it is difficult to properly separate data into components.

Shaders for example, you can make it much more flexible. Every entity could have a shader component attached to it.

But for the shader like the menu one, it should be a built-in shader in the rendering system, in my opinion. The shader itself is not really an entity. We could do something like where the rendering system has three built-in shaders:

  • Game Shader
  • UI Shader
  • General Shader

And set these shaders from outside.

— Reply to this email directly or view it on GitHub https://github.com/Sherushe/tanks/issues/38#issuecomment-67811191.

ghost commented 9 years ago

You need to have one thing in mind. We won't make 100 components types in the final game, even the current components need to be grouped like you said before. So making a wrapper to make components easier is superficial in my opinion.

But for the design, here are the problems I see:

So design-wise, it is not really problematic, but try for yourself. I won't change anything for the next month.

Also Components =/= member variable in OOP. For the shader, it's a proposition, but I would define a Shader Component which contains the sf::Shader and a vector of parameters passed to the shader and I would not even create an entity for this shader but directly pass it to the rendering system because it's not an entity, it's a feature of the rendering system.

ghost commented 9 years ago

I dont see how checking for text is heavy if it is already looping through it. Checking for text becomes heaving when searching for it, not checking against it. You are a bit right with having a bit more errors concerning the text names. I will see if that can be somehow dealt with. I realize that components are not variables but more like placeholders to tell one of the systems that the entity has "this". However, it is possible to achieve the same without changing much of the core design and minimal change to the entities.

Things that will need to be refactored:

The Mouse and Keyboard Controllers I made can be global instead of components. Would it make sense to put them as part of the Input system?

I will look into having the predefined shader component. I guess initializing the shader on the fly is not efficient.

The UI code in the UI system's constructor will go into the UI manager.

... and other things can't think of at this moment.

ghost commented 9 years ago

You're still searching for it. For example, if you want to check if the "visible" component is in the entity, you have to loop though all the components the entity has, and check if there is a match. But it will slow down the process not crash it so it's okay I guess, we still have a margin on the fps side.

Things that will be refactored, sounds good. Yeah the keyboard and the mouse states should be part of the input system itself.

ghost commented 9 years ago

But it will slow down the process not crash it so it's okay I guess, we still have a margin on the fps side.

In that case, set a limit. What value should the FPS never go below? I would say 120 fps because that's the human perceiving limit as well as the max on mostly any monitor you would come across.

On Tue, Dec 23, 2014 at 3:52 AM, Sherushe notifications@github.com wrote:

You're still searching for it. For example, if you want to check if the "visible" component is in the entity, you have to loop though all the components the entity has, and check if there is a match. But it will slow down the process not crash it so it's okay I guess, we still have a margin on the fps side.

Things that will be refactored, sounds good. Yeah the keyboard and the mouse states should be part of the input system itself.

— Reply to this email directly or view it on GitHub https://github.com/Sherushe/tanks/issues/38#issuecomment-67931938.

ghost commented 9 years ago

Yeah, it should never go under 120 fps, if it goes under that we either need to remove the feature slowing down the process or optimize.

During the whole dev stage, it should never go under 120 fps. When we get to the optimize step, hopefully we can rise that number even higher. But we shouldn't say "it's 30fps now but after all the optimizations it will be fine" unless it's an exception case.

ghost commented 9 years ago

How do you access members of a manager from a certain system? For example, I made an InputController Manager, declared in Application and want to access its members from the Input System.

On Tue, Dec 23, 2014 at 5:59 PM, Sherushe notifications@github.com wrote:

Yeah, it should never go under 120 fps, if it goes under that we either need to remove the feature slowing down the process or optimize.

During the whole dev stage, it should never go under 120 fps. When we get to the optimize step, hopefully we can rise that number even higher. But we shouldn't say "it's 30fps now but after all the optimizations it will be fine" unless it's an exception case.

— Reply to this email directly or view it on GitHub https://github.com/Sherushe/tanks/issues/38#issuecomment-68008195.

ghost commented 9 years ago

I made the InputController Manager global if that's ok because I need to access the members from 2 systems

ghost commented 9 years ago

Making managers requires making some variables such as boolean fullscreen global. It makes sense since it is environment-wide, but I don't know if you approve.

ghost commented 9 years ago

Indeed, but the problem is not that. Inputs should not really be managed by a manager. It should pass though the event system in my opinion.

Input handling is not really resource loading or data intialization.

ghost commented 9 years ago

I don't really see how it could be dealt with events since it's not just one thing that needs to be done. The Input handler input states that are to be accessed, and not run. I see events as something that need to be executed, not something to pass data from one place to another.

On Wed, Dec 24, 2014 at 3:18 AM, Sherushe notifications@github.com wrote:

Indeed, but the problem is not that. Inputs should not really be managed by a manager. It should pass though the event system in my opinion.

Input handling is not really resource loading or data intialization.

— Reply to this email directly or view it on GitHub https://github.com/Sherushe/tanks/issues/38#issuecomment-68035651.

ghost commented 9 years ago

I see events as something that need to be executed, not something to pass data from one place to another.

It's a bit problematic, if we have different definitions for word. But the official defintion for events is neither. The event is the data itself that is passed. The callback is what is executed. I'm not making this up look at the defintion : http://en.wikipedia.org/wiki/Event_(computing) In Irrlicht, I'm sure events are also data that is passed from one place to another ( the irrlicht library to the game program ).

When a system reads the events it doesn't erase them. So multiple systems can react to one event sent from the input system.

So what I had in mind was:

For the mouse coordinates sfml already provides a way to get the coordinates at a global scope.

This KeyEvent or MouseEvent can then be processed by the systems if needed.

ghost commented 9 years ago

When a system reads the events it doesn't erase them. So multiple systems can react to one event sent from the input system.

So what I had in mind was:

  • When a key is pressed => KeyEvent is sent
  • When the mouse is clicked => MouseEvent is sent

Perhaps we can just extend SFML's event class and do it that way? Though I don't think it is meant to be extended unlike Irrlitch's. You would create an instance of the event handler to be managed by the window, where this is the callback that you override which would execute:

OnEvent(const SEvent& event)

This would be called internally within Irrlicht's system and operate on local or global variables within the scope of the Event's extended class . I never used events to pass values as they have no parameters. I either declared varaibles globally, passed them through parameters, or made it so the classes I'm dealing with had access to the variables. I also remember making an entire system using Irrlicht's events handling scheme last year using what I call "Actions", I don't remember exactly what I did, but I can look it up if you want more details.

For the mouse coordinates sfml already provides a way to get the coordinates at a global scope.

Yea, that's how I get mouse coordinates. However, the Input Controller adds additional states such as click states and press states. While press states are same as mouse buttons, it is required to act as a buffer to determine the previous press state to determine the click state.

I was thinking on creating a dynamic variable class that keeps track of it's previous value, but always had second thoughts on implementing it. It would help with the mouse and key states as well as checking for things such as if the game changed from windowed to fullscreen or the other way around.

On Thu, Dec 25, 2014 at 4:10 PM, Sherushe notifications@github.com wrote:

I see events as something that need to be executed, not something to pass data from one place to another.

It's a bit problematic, if we have different definitions for word. But the official defintion for events is neither. The event is the data itself that is passed. The callback is what is executed. I'm not making this up look at the defintion : http://en.wikipedia.org/wiki/Event_(computing) In Irrlicht, I'm sure events are also data that is passed from one place to another ( the irrlicht library to the game program ).

When a system reads the events it doesn't erase them. So multiple systems can react to one event sent from the input system.

So what I had in mind was:

  • When a key is pressed => KeyEvent is sent
  • When the mouse is clicked => MouseEvent is sent

For the mouse coordinates sfml already provides a way to get the coordinates at a global scope.

This KeyEvent or MouseEvent can then be processed by the systems if needed.

— Reply to this email directly or view it on GitHub https://github.com/Sherushe/tanks/issues/38#issuecomment-68113092.

ghost commented 9 years ago

You should really test the things you're saying or at least imagine how it would work. Maybe ithose are good ideas.

PS: I'm saying this because I can't really debate design ideas right now, I have too much things in mind. Like I said, do whatever you like while I'm away. I will write an exhaustive feedback when I'm back.

ghost commented 9 years ago

I am running into problems with events yet again. I fixed the issue last time by making sure the events are dealt with at least once before being cleared, but now I need the events be used by multiple systems. As a result, the event gets used by one system and gets cleared before the other system gets a chance to use it. I have an idea on how to fix it, but I'm too scared to touch the Event code myself as I don't feel 100% confident I won't break it in the process. But the idea is simple: have something tracking the use of the events by other systems. The implementation would look something like this:

emit(T* t, Sys*)

Where Sys is the list of systems this event is targeted for. I would work much how the components are checked for:

env->hasComponents<TankControls, Velocity>

It's use would be in a system like so:

env->getEvents<MenuEvent>(this)

Where this would be the pointer to the system to identify that it is the target system.

Then when all targeted systems have used the event once, it can be cleared.

If you think this is a good idea, then we can get rid of the temporary solution with the eventExec bit field I set up a while back.

ghost commented 9 years ago

No good. The UI System's usage of events is now broken. The clearing event bug still remains. https://github.com/Sherushe/tanks/issues/59

ghost commented 9 years ago

I will check the new commit, but for some reason there werent any events at that point. I find it very strange that it works on your side and not on mine.

ghost commented 9 years ago

I think I see where the problem lies. The UI system has 2 different events it gets, but it emits one of the events it gets at the end of its update routine. The other event gets emitted from the UI Manager. As a result there are 2 visual problems: The menu is still shown when it needs to be hidden, and the game entities don't reset when the New Game button is pressed.

ghost commented 9 years ago

Found a bug in the Environment system. When creating an entity and using the pointer of a component from another entity as one of its components, deleting one of the said entities ensures a dangling pointer to the component of the other entity. Can there be support for shared components among entities?

ghost commented 9 years ago

I guess. Right now, I don't see a problem doing that.