RangeVisualization component and use it to visualize Aura ranges.
AuditedrP19519
Actions

Description

RangeVisualization component and use it to visualize Aura ranges.

Differential Revision: https://code.wildfiregames.com/D238
Fixes #4349
Patch By: Sandarac

Details

Auditors

mimo

Committed

elexis

May 6 2017, 2:47 AM

Differential Revision

D238: Visual Aura Range

Parents

rP19518: Select the number of relics to spawn in the gamesetup.

Branches

Unknown

Tags

Unknown

Build Status

Buildable 1479
Build 2336: Trigger Windows Autobuild
Build 2335: Post-Commit Build	Jenkins

Event Timeline

elexis committed rP19519: RangeVisualization component and use it to visualize Aura ranges..May 6 2017, 2:47 AM

Harbormaster completed building B1479: rP19519: RangeVisualization component and use it to visualize Aura ranges..May 6 2017, 4:08 AM

Sandarac mentioned this in D432: Visual Heal Range.May 6 2017, 8:16 AM

elexis added inline comments.May 27 2017, 10:07 PM

/ps/trunk/source/simulation2/components/CCmpSelectable.cpp
657	This fixes #4260

Wasdat? Missing a step in a for-loop?

fatherbushido added a subscriber: fatherbushido.Jun 12 2017, 9:29 AM

fatherbushido added inline comments.

/ps/trunk/source/simulation2/components/CCmpSelectable.cpp
480	Why did you choose to remove that check?
486	The comment was not so bad

fatherbushido added inline comments.Jun 12 2017, 1:56 PM

/ps/trunk/source/simulation2/components/CCmpSelectable.cpp
480	uhm I see, as we add the thickness, it won't never be the case

fatherbushido added inline comments.Jun 12 2017, 2:35 PM

/ps/trunk/source/simulation2/components/CCmpSelectable.cpp
486	Ok we see overlay.m_Closed = true; some lines above

There is a small GUI bug - if an non-owned entity is selected and captured, the overlay is not shown until selecting the thing again.

mimo raised a concern with this commit.Aug 24 2017, 7:11 PM

mimo added a subscriber: mimo.

mimo added inline comments.

/ps/trunk/binaries/data/mods/public/gui/session/session.js
795	Maybe i missed something, but this GuiInterfaceCall on each turn looks like overkill, especially as now we do it also for heal range, and other cases will certainly be added in the future. Wouldn't it be better that each time one option is changed in the option panel, the gui receives a notification with the name of the option changed, and takes action accordingly?

This commit now has outstanding concerns.Aug 24 2017, 7:11 PM

elexis added inline comments.Aug 25 2017, 5:34 AM

/ps/trunk/binaries/data/mods/public/gui/session/session.js
795	Don't recall if it's only used for reloading the configuration or whether there was some weird edge case. Performance cost isn't intense as it just sets one variable, but it's indeed messy to do it in this function. Only the foremost GUI page gets code exection (so no instant feedback). Should be sufficient to just send a callback (f.e. the aiconfig dialog uses that) informing that the options changed when the options page is closed. Collecting the changes config values seems unneeded atm.

elexis mentioned this in rP20622: Render the range visualization of auras, heal and attack component in a….Dec 10 2017, 3:42 AM

elexis mentioned this in rP20627: Rename RangeVisualization component from rP19519 / D238 to RangeOverlayManager….Dec 10 2017, 4:29 AM

temple mentioned this in D1291: Put diplomacy colors in options.Feb 11 2018, 5:59 AM

elexis mentioned this in rP21527: Implement basic color editing in the options page and use it for the diplomacy….Mar 12 2018, 5:49 AM

That GUIInterface done only on init and when closing the options dialog following rP21527.

This commit now requires verification by auditors.Mar 12 2018, 5:49 AM

ping @mimo

ok looks good (although i have not tested it in game)

All concerns with this commit have now been addressed.Mar 13 2018, 7:01 PM

vladislavbelov added a subscriber: vladislavbelov.Dec 11 2019, 9:51 AM

vladislavbelov added inline comments.

/ps/trunk/source/simulation2/components/CCmpSelectable.cpp
264	This method should be marked as virtual to prevent overloading or missed usage.
309	It should be `TEXTURE_BASE_PATH` as all other constants, the current version is less readable.