MichaelAquilina / Some-2D-RPG

Tinkering with Game Development
MIT License
28 stars 5 forks source link

Added an Animation cache #8

Closed behindcurtain3 closed 11 years ago

behindcurtain3 commented 11 years ago

So files don't have to be reloaded each time they are requested, includes a metric to track how many file loads this saves.

MichaelAquilina commented 11 years ago

While the idea behind this change is good, i think there are a few issues with it. For example, the cache should store Animation instances and not GameDrawableInstance objects because otherwise two Entities that use the same Animation will end up being synchronized instead of allowing for seperate animation states etc... I will give this a further look later on and see if it can be changed to suite its intended purpose.

Something of note however, i try avoid Caching objects during an early development lifecycle because it can cause a lot of future bugs. Caching is a form of optimization technique and there is a famous saying that goes "Premature optimization is the root of all evil", and from my experience this has always proved to be true.

behindcurtain3 commented 11 years ago

Yeah I see your point about the early optimization. This can probably just be closed and revisited later if needed. As a side note is there a reason the GameDrawableInstance returned from the DrawableSet Add function has its layer set again in the Animation loading? It looks like it might be redundant.

MichaelAquilina commented 11 years ago

The reason why a the Add method in DrawableSet returns a GameDrawableInstance is because when you add a drawable item to some Entity's drawables, you want a copy of that drawable so that the entity can set attributes that alter the drawable without altering any other entity that uses the same drawable. For example, if i have mulitple bats which use the same Animaton and i wish to change one bat to have its Animation tinted red, then i would need a separate instance for each one in order for the tinting to not affect all other bats using the same Animation. This is the idea behing the GameDrawableInstance class.

For this reason, when you add a drawable to a DrawableSet, it is wrapped around a GameDrawableInstance that refers to that animation and provides properties that can be set such as its color tint, visibility, opacity etc..... This GameDrawableInstance that is wrapping the Animation is then returned by the Add method so that any alterations can be made as necessary after the call has been made.

Its definitely not obvious and not straightforward by any means - so documentation should be added to that method as soon as possible. Ill be trying to update the wiki to provide more detail on this too (there is currently some information on it thought if you wish to take a look).

Hope this helps.

behindcurtain3 commented 11 years ago

Thanks, I understand the principal behind it but it seems like the Layer property is set twice on the new GameDrawableInstance. First on line 68 in the DrawableSet class and then again lines 159 and 168 on the Animation class once the new GameDrawableInstance has been returned. I'm just confused because the layer is passed as an argument and then set on the object that is going to be returned and then set again on the method its returned to. Sorry, this seems really minor now that I'm writing this out but I wanted to ask.

MichaelAquilina commented 11 years ago

Its probably some code that i forgot to clean up when i was making changes. Ill write it down on my todo list to take a look. Nothing is minor during the development life cycle so anything pointed out is all good :)

MichaelAquilina commented 11 years ago

So i took a look at the Animation class quickly. The reason why Layer is set twice is because its a manual error on my part. The setting of the layer value does not seem to be set on this line and this line. I will go ahead and clean it up at some point.

MichaelAquilina commented 11 years ago

Just thought id give an update, once i add a proper Content Pipeline extension for loading tmx, tsx and anim files - then all loading will be handled by the content pipeline and thus be cached internally. Making use of the content pipeline to load stuff also makes reading from xml files much much faster. You can see more details here: http://msdn.microsoft.com/en-us/library/bb447745.aspx