Page MenuHomeWildfire Games

Remove useless computations and simplify code for getting the points of some lines in Selectable component.
ClosedPublic

Authored by fatherbushido on Jun 23 2017, 4:33 PM.

Details

Summary

Some helpers functions are currently useless and leads to computations of arccos (with many rounding errors).
The aim of the patch is to remove that and simplify the code a bit.

WIP:

  • I would perhaps write a test or something like that
  • Need to check the types things
  • We could compute only one cos - sin and then only use multiplications but with compounded approx we have sometimes weird results, so I prefer skip that for the moment
Test Plan
  • Compilation
  • Code
  • Check that the helpers are not used anywhere else
  • In game: health or aura range ring and selection ring of round-footprint building

(I put leper as subscribers so he can hit me for my bad cpp ;-))

Event Timeline

fatherbushido created this revision.Jun 23 2017, 4:33 PM
fatherbushido edited the summary of this revision. (Show Details)Jun 23 2017, 4:41 PM
Vulcan added a subscriber: Vulcan.Jun 23 2017, 5:22 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1613/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/230/ for more details.

Sandarac resigned from this revision.Jun 24 2017, 4:56 PM

I don't really know this code; when adding the range visualization I just moved the code that was already in the old UpdateStaticOverlay into the new UpdateTexturedOverlay (I didn't change/look at the math).

I don't really know this code; when adding the range visualization I just moved the code that was already in the old UpdateStaticOverlay into the new UpdateTexturedOverlay (I didn't change/look at the math).

:/

Itms edited edge metadata.Aug 3 2017, 12:35 AM

It does seem useless to have so much code for cutting a circle into small chords ?
I think refactoring that code makes it truly cleaner.

Your cpp is not bad ? I'm sorry I'm not going to compile and test but you can consider the math and general proposal reviewed.

source/simulation2/components/CCmpSelectable.cpp
512

I'd write float(2 * M_PI) for consistency with the computation of numSteps.

Thanks for having looked at it Itms!

This revision was automatically updated to reflect the committed changes.