search

Unity-Technologies / Animation-Instancing

This technique is designed to instance Characters(SkinnedMeshRender).
Other
1.63k stars 296 forks source link

[Feedback]Undocumented Code #96

Open LaireonGames opened 3 years ago

LaireonGames commented 3 years ago

Hey folks,

Thanks for posting this project and the work done on it but I do feel it really could do with some better in code documentation. So much in this project is superfluous to the needs of my project (which is always the case going from wide use cases to specific ones) so I'm trying to grab only what I know to be relevant.

As a bit of feedback, doing this was a really tedious process because of the state of the code base and lack of comments, regions, etc. It was confusing trying to figure out what each section was doing and what the aim was as well as why there are so many sub classes and what the purpose of each one was.

Variable/method names could also do with another think. E.G I found this method in a shader:

loadMatFromTexture

My first thought was "eh, load material from texture? That makes no sense can't be that". Then I have to investigate and realise ah course its Matrix.

Another example was a class RuntimeHelper. So far as I can see nothing in there is actually used at runtime.

Not going to list them all and I've made too many changes to commit anything back to the repo but for the sake of the next person please consider giving the repo another pass for comments etc

ashwin911 commented 3 years ago

This repository is pretty much unmaintained. I've submitted a pull request months ago which fixed a pretty significant bug and it still hasn't been reviewed. So basically typical Unity developer behaviour, releasing shiny new features in a broken state and then cease any communication about the state of the features with the customers.