ashish2199 / Aidos

A beginner friendly project with aim of creating our own version of Bomberman.
GNU General Public License v3.0
41 stars 27 forks source link

Changes to Sprite classs #28

Closed ItsStolas closed 6 years ago

ItsStolas commented 6 years ago

@ashish2199 you've changed the Renderer to need to take a player, rather than how I had it set up using the Sprites X and Y. This makes no sense at all to me. The way I had it, it was generic. Every sprite could be assigned a position. Now the renderer is hugely dependant on the Player class.

Can you explain these changes?

Did you also want to submit pull requests for major changes, just so we can all discuss them? I'm unsure of the mentality for this project. IE, is it a total group effort. Or is it more a project that you own, and we are contributing too?

Thanks Corey

ItsStolas commented 6 years ago

Added a pull request with a more generic renderer.

ashish2199 commented 6 years ago

@CoreyHendrey Yeah, what I had done had made sprite class less generic and brittle. Sprite should not depend on player class.

What I wanted to actually do was to remove the sprite from having position instance variables as I see sprite to be used for representing a entity on canvas and for animation only.

Sprites having instance variables to represent their position doesn't make sense to me as their position is the same as position of the entities they represent. Also we need to make them in sync after every move which I feel should be avoided. What do you think about it ?

I suggest we should have methods like getPositionX( ) , getPositionY( ) to get position of the entity we need to draw, Added to entity interface. we can then use these methods instead of sprite's x and y. Also so far to me I feel entity is anything that is drawn on screen, hence adding these methods to get their position on screen dont seem to do any harm.

ItsStolas commented 6 years ago

I struggled with that too :) I think your way would be great. As long as the Sprite class stays independent of derivatives of Entity, and just draws either Entity, or preferably just draws sprites. We could use it for the menu and GUI and stuff if we keep it separate from Entity, but I'm sure we could do the menu another way too.

ashish2199 commented 6 years ago

Added a entity reference to sprite class which will now give the position for that sprite. We would now need a entity to create a sprite.