brunomikoski / Animation-Sequencer

A visual tool that allows you to create animated sequences of tweens and tweak them on editor time.
MIT License
960 stars 116 forks source link

feature/defaults_graphic_workaround_speed_and_bug_fixes #36

Closed nindim closed 2 years ago

nindim commented 2 years ago
brunomikoski commented 2 years ago

One small question and some weird line ending! but overall pretty good improvements! Thanks a lot for the contribution again!

I also advise you to take a look at the warnings/errors on the codacy! https://github.com/brunomikoski/Animation-Sequencer/pull/36#pullrequestreview-835851828

Its not always mandatory but its good to keep an eye on it!

nindim commented 2 years ago

Hey @brunomikoski,

I had a look through the Codacy issues, please see below:

"Remove this initialization to 'XXX', the compiler will do that for you." I matched the style of the existing code when setting bools to false (it already existed for useRelative).

"Remove this unused method parameter 'gameObject'." The problem it's picking up with the PrefabSaving(GameObject gameObject), this can't be removed as it's a Unity provided Action.

"Duplicated block occurs 2 times in 1 files" With regards to the the duplication Codacy picking up between FadeGraphicDOTweenAction and ColorGraphicDOTweenAction, given it's a (hopefully) temporary workaround, is this OK? There is no common base class in which the Graphic component is available so no clean place to put a shared function. Any ideas here?

With the above in mind, is there anything I need to do here to get the PR mergeable?

Thanks,

Niall