search

enginmanap / limonEngine

3D FPS game engine with full dynamic lighting and shadows
http://www.limonengine.com
GNU Lesser General Public License v3.0
580 stars 57 forks source link

[issue #54] refactored calculateTransform #61

Closed nickshillingford closed 6 years ago

enginmanap commented 6 years ago

Hi!

Thanks for the PR. I don't think it will compile tough, because the interface is implemented by some classes, and they need to be updated. Then the usage of those should be updated too. I believe compiler will guide you with errors.

nickshillingford commented 6 years ago

Okay, sounds good. I wasn't sure if you wanted the implementations to be updated as well for this particular issue. I'll update them and submit another pull request.

nickshillingford commented 6 years ago

Hey,

I updated all the classes I could find that implement the interface, as well as all the use cases of the method I could find. Compiled for me.

Also, I changed the bool name from "isFound" to "status" since you said that it should return the status.

nickshillingford commented 6 years ago

Hmm, I don't know why AnimationSequencer didn't appear when I did a scan of the repository for calculateTransform uses. I'm sorry about missing that.

I am not at home currently but I will definitely fix that and remove the unnecessary code by the end of the day. Is that okay?

enginmanap commented 6 years ago

No need to rush, you can do whenever you like :)

nickshillingford commented 6 years ago

Fixed AnimationSequencer.cpp, removed the unnecessary bool variable from World.cpp and the map search from AnimationCustom.cpp, since there is no map instance for that class.

(I meant to say unnecessary, not necessary, in the description for commit 58e7b09)

Everything should work now. Is it good?

enginmanap commented 6 years ago

Hey,

I just merged it, great work!

nickshillingford commented 6 years ago

Awesome!