Page MenuHomeWildfire Games
Differential D1509

Page Down / Page Up Rotates Entities in Editor Even Without Focus
ClosedPublic
Actions

Authored by Silier on May 19 2018, 7:14 PM.

Details

Reviewers
Itms
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22125: Rotate entities during placement in Atlas with the keyboard only when the…
Summary

Reported on forum: https://wildfiregames.com/forum/index.php?/topic/24358-page-down-page-up-rotates-entities-in-editor-even-without-focus/

Rotation of entity while placing can be done by page up and page down keys and it is hold from onTick function, what means there is no control if window is active.

Test Plan

with unfocused window entities does not rotate anymore with pgup and pgdown

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Silier created this revision.May 19 2018, 7:14 PM
Owners added a subscriber: Restricted Owners Package.May 19 2018, 7:14 PM
Silier edited the summary of this revision. (Show Details)May 19 2018, 7:15 PM
Vulcan added a subscriber: Vulcan.May 19 2018, 7:19 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/522/display/redirect

Harbormaster completed remote builds in B6095: Diff 6584.May 19 2018, 7:20 PM
vladislavbelov added a subscriber: vladislavbelov.EditedMay 19 2018, 7:43 PM

It's not actually correct, it depends on FPS now. To process it correctly you need to store key presses and handle them inside OnTick, counting the dt.

Silier updated this revision to Diff 6585.May 19 2018, 8:57 PM
Silier edited the summary of this revision. (Show Details)

new variant, that tries to match @vladislavbelov comment

Silier added inline comments.May 19 2018, 8:59 PM
source/tools/atlas/AtlasUI/ScenarioEditor/Tools/PlaceObject.cpp
132 ↗(On Diff #6585)

note: not matching direction or comment

Vulcan added a comment.May 19 2018, 9:01 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/523/display/redirect

Harbormaster completed remote builds in B6096: Diff 6585.May 19 2018, 9:01 PM
vladislavbelov added inline comments.May 19 2018, 9:20 PM
source/tools/atlas/AtlasUI/ScenarioEditor/Tools/PlaceObject.cpp
45 ↗(On Diff #6585)

The member initialisation should be inside ctor params:

PlaceObject()
    m_RotationLeft(false), m_RotationRight(false)
{
    ...
}
132 ↗(On Diff #6585)

Also put expressions on new lines, since you're here.

140 ↗(On Diff #6585)

It doesn't seem right. You need to reset them on the key up event, not here.

Silier updated this revision to Diff 6586.May 20 2018, 8:56 AM

Key up, key down

Vulcan added a comment.May 20 2018, 9:03 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/524/display/redirect

Harbormaster completed remote builds in B6097: Diff 6586.May 20 2018, 9:03 AM
Silier added a comment.Jan 13 2019, 6:13 PM

can someone take look at this ?

wraitii added a reviewer: Restricted Owners Package.Jan 14 2019, 12:08 PM
Itms requested changes to this revision.Mar 17 2019, 3:45 PM
Itms added a subscriber: Itms.

The patch works for me! And I agree with the implementation. May I suggest that, instead of adding those two new booleans, you make the local dir variable in RotateTick an attribute of the class? You can rename it m_RotationDirection and modify it directly in OnKeyOverride. That would make the patch shorter and the code cleaner.

This revision now requires changes to proceed.Mar 17 2019, 3:45 PM
Silier updated this revision to Diff 7567.Mar 17 2019, 4:11 PM

2 variables -> one

Silier updated this revision to Diff 7568.Mar 17 2019, 4:13 PM

missing dir

Vulcan added a comment.Mar 17 2019, 4:13 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/tools/atlas/AtlasUI/ScenarioEditor/Tools/PlaceObject.cpp
|   1| /*·Copyright·(C)·2013·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2013"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1104/display/redirect

Harbormaster completed remote builds in B6992: Diff 7567.Mar 17 2019, 4:13 PM
Vulcan added a comment.Mar 17 2019, 4:16 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/tools/atlas/AtlasUI/ScenarioEditor/Tools/PlaceObject.cpp
|   1| /*·Copyright·(C)·2013·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2013"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1105/display/redirect

Harbormaster completed remote builds in B6993: Diff 7568.Mar 17 2019, 4:16 PM
Silier updated this revision to Diff 7569.Mar 17 2019, 4:31 PM

less lines

Vulcan added a comment.Mar 17 2019, 4:33 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1106/display/redirect

Harbormaster completed remote builds in B6994: Diff 7569.Mar 17 2019, 4:33 PM
This revision was not accepted when it landed; it landed in state Needs Review.Mar 17 2019, 5:04 PM
Closed by commit rP22125: Rotate entities during placement in Atlas with the keyboard only when the… (authored by Itms). · Explain Why
This revision was automatically updated to reflect the committed changes.
Itms added a comment.Mar 17 2019, 5:05 PM

Oops I forgot to accept first. Seems like I can't now :(

elexis added a subscriber: elexis.Jun 22 2019, 6:30 PM

(Might be worth a comment in the code if this is a common pitfall where to put the key code)

Revision Contents
Changeset List

PathSize
ps/
trunk/
source/
tools/
atlas/
AtlasUI/
ScenarioEditor/
Tools/
32 lines

Diff 7574

View Options

ps/trunk/source/tools/atlas/AtlasUI/ScenarioEditor/Tools/PlaceObject.cpp

Loading...
Wildfire Games · Phabricator