0ad/0ad
0ad/0ad
11
9
Fork 21
Code Issues 1.5k Pull Requests 36 Actions Projects Releases Wiki Activity

Left-click the portrait to follow the entity #7018

Merged
sera merged 2 commits from abian/0ad:left-click-portrait into main 2024-09-18 06:47:00 +02:00
Conversation 4 Commits 2 Files Changed 6 +44 -19
abian commented 2024-09-02 13:23:01 +02:00
Member
Copy Link

Left-clicking the portrait of a unit will make the camera follow that unit (before: no action). A tooltip informs the player of this possibility.
Left-clicking the portrait of a structure will make the camera focus on that structure (before: no action). A tooltip informs the player of this possibility.
Double-clicking a hero/treasure icon will make the camera follow that hero/treasure (before: just focus on that hero/treasure).
Some minor related changes.

Fixes #6879

Left-clicking the portrait of a unit will make the camera follow that unit (before: no action). A tooltip informs the player of this possibility. Left-clicking the portrait of a structure will make the camera focus on that structure (before: no action). A tooltip informs the player of this possibility. Double-clicking a hero/treasure icon will make the camera follow that hero/treasure (before: just focus on that hero/treasure). Some minor related changes. Fixes #6879
abian added 1 commit 2024-09-02 13:23:04 +02:00
Left-click the portrait to follow the entity
All checks were successful
0ad-freebsd/pipeline/pr-main This commit looks good
Details
checkrefs / checkrefs (pull_request) Successful in 58s
Details
pre-commit / build (pull_request) Successful in 1m10s
Details
0ad-windows/pipeline/pr-main This commit looks good
Details
0ad-linux/pipeline/pr-main This commit looks good
Details
0ad-macos/pipeline/pr-main This commit looks good
Details
1411c27082 
Left-clicking the portrait of a unit will make the camera follow that unit (before: no action). A tooltip informs the player of this possibility.
Left-clicking the portrait of a structure will make the camera focus on that structure (before: no action). A tooltip informs the player of this possibility.
Double-clicking a hero/treasure icon will make the camera follow that hero/treasure (before: just focus on that hero/treasure).
Some minor related changes.

Fixes #6879
abian requested review from wraitii 2024-09-02 13:23:04 +02:00
abian requested review from marder 2024-09-02 13:23:04 +02:00
abian requested review from Imarok 2024-09-02 13:23:04 +02:00
abian requested review from s0600204 2024-09-02 13:23:04 +02:00
abian removed review request for wraitii 2024-09-02 13:24:12 +02:00
abian removed review request for marder 2024-09-02 13:24:13 +02:00
abian removed review request for Imarok 2024-09-02 13:24:15 +02:00
abian removed review request for s0600204 2024-09-02 13:24:16 +02:00
abian requested review from sera 2024-09-02 13:24:22 +02:00
abian requested review from Stan 2024-09-02 13:24:22 +02:00
sera reviewed 2024-09-02 13:45:04 +02:00
sera left a comment
Member
Copy Link

Being pedantic here, about commit messages: https://www.baeldung.com/ops/git-commit-messages#3-the-5072-rule. ducks

Except for nitpicks this looks fine and the functionality useful.

Being pedantic here, about commit messages: https://www.baeldung.com/ops/git-commit-messages#3-the-5072-rule. *ducks* Except for nitpicks this looks fine and the functionality useful.
πŸ‘ 1
binaries/data/mods/public/gui/session/selection_details.js Outdated
@ -362,2 +370,4 @@
getTreasureTooltip
].map(func => func(template)));
let leftClickTooltip = hasClass(entState, "Unit") ? getFollowOnLeftClickTooltip() : getFocusOnLeftClickTooltip();
sera commented 2024-09-02 13:37:03 +02:00
Member
Copy Link

looks like it could be const, seems the linter isn't up and running just yet.

looks like it could be const, seems the linter isn't up and running just yet.
πŸ‘ 1
abian commented 2024-09-02 13:55:39 +02:00
Author
Member
Copy Link

Should I add another commit? Or squash? Let's see if I don't mess it up once again...

Should I add another commit? Or squash? Let's see if I don't mess it up once again...
sera commented 2024-09-02 14:06:54 +02:00
Member
Copy Link

During review it's fine to add just another commit on top and can in some scenarios help the review process but before merging squashing is a must. For the sake of testing squashing and force pushing I'd say just try it out and see what happens.

During review it's fine to add just another commit on top and can in some scenarios help the review process but before merging squashing is a must. For the sake of testing squashing and force pushing I'd say just try it out and see what happens.
abian commented 2024-09-02 14:24:27 +02:00
Author
Member
Copy Link

It worked! \o/

It worked! \o/
abian marked this conversation as resolved
binaries/data/mods/public/gui/session/selection_panels_helpers.js Outdated
@ -386,14 +386,16 @@ function cancelUpgradeEntity()
}
/**
* Set the camera to follow the given entity if it's a unit.
sera commented 2024-09-02 13:34:22 +02:00
Member
Copy Link

An entity can be a building or even a tree, therefore the specialization of unit

An entity can be a building or even a tree, therefore the specialization of unit
abian commented 2024-09-02 13:52:35 +02:00
Author
Member
Copy Link

I changed the word as it can now "follow" all kinds of entities (the camera will just focus on buildings or trees, but there's no noticeable difference).

I changed the word as it can now "follow" all kinds of entities (the camera will just focus on buildings or trees, but there's no noticeable difference).
sera commented 2024-09-02 14:07:57 +02:00
Member
Copy Link

So you set the camera to the position of entity if possible and enable follow if entity is unit

So you set the camera to the position of entity if possible and enable follow if entity is unit
abian marked this conversation as resolved
abian commented 2024-09-02 13:53:00 +02:00
Author
Member
Copy Link

Thank you for your comments and patience! πŸ˜„

Thank you for your comments and patience! πŸ˜„
abian force-pushed left-click-portrait from 1411c27082 to 37e69cae55 2024-09-02 14:22:34 +02:00 Compare
Vantha commented 2024-09-03 23:23:46 +02:00
Member
Copy Link

From quickly checking over it, this looks good to me. Cool.

From quickly checking over it, this looks good to me. Cool.
πŸŽ‰ 1
Vantha reviewed 2024-09-03 23:24:40 +02:00
Vantha left a comment
Member
Copy Link

Just one small thing;)

Just one small thing;)
binaries/data/mods/public/gui/session/selection_details.js Outdated
@ -325,3 +325,3 @@
Engine.GetGUIObjectByName("icon").sprite = template.icon ? ("stretched:session/portraits/" + template.icon) : "BackgroundBlack";
if (template.icon)
Engine.GetGUIObjectByName("iconBorder").onPressRight = () => {
{
Vantha commented 2024-09-03 23:23:57 +02:00
Member
Copy Link

Sorry if I'm being overly nitpicky here, but I believe it's customary in this codebase to place the opening brace on the same line as the lamba expression for arrow functions.

Sorry if I'm being overly nitpicky here, but I believe it's customary in this codebase to place the opening brace on the same line as the lamba expression for arrow functions.
abian commented 2024-09-04 00:28:58 +02:00
Author
Member
Copy Link

Oh, maybe the diff is confusing. This line used to open a lambda expression, but now it opens the if block.

Oh, maybe the diff is confusing. This line used to open a lambda expression, but now it opens the if block.
Vantha commented 2024-09-04 11:07:47 +02:00
Member
Copy Link

Ah, I see. Everything's fine then, my bad.

Ah, I see. Everything's fine then, my bad.
abian commented 2024-09-05 21:19:56 +02:00
Author
Member
Copy Link

No worries!

No worries!
abian marked this conversation as resolved
sera approved these changes 2024-09-07 11:02:31 +02:00
sera left a comment
Member
Copy Link

tested, works as promised

tested, works as promised
binaries/data/mods/public/gui/session/selection_details.js Outdated
@ -328,0 +327,4 @@
{
const iconBorder = Engine.GetGUIObjectByName("iconBorder");
// Actions on left click
sera commented 2024-09-07 11:02:19 +02:00
Member
Copy Link

Guess the comment here isn't really needed

Guess the comment here isn't really needed
abian commented 2024-09-07 11:10:26 +02:00
Author
Member
Copy Link

Agreed, removed! Thanks for the testing and for moving this forward.

Agreed, removed! Thanks for the testing and for moving this forward.
abian marked this conversation as resolved
abian added 1 commit 2024-09-07 11:09:46 +02:00
Remove unnecessary comments in selection_details.js
Some checks reported warnings
0ad-freebsd/pipeline/pr-main This commit looks good
Details
checkrefs / checkrefs (pull_request) Successful in 57s
Details
pre-commit / build (pull_request) Successful in 1m14s
Details
0ad-windows/pipeline/pr-main This commit looks good
Details
0ad-linux/pipeline/pr-main This commit looks good
Details
0ad-macos/pipeline/pr-main This commit is unstable
Details
eb3547c7c0 
Related: #7012
abian force-pushed left-click-portrait from eb3547c7c0 to 2cae398a6f 2024-09-10 13:10:30 +02:00 Compare
abian force-pushed left-click-portrait from 2cae398a6f to d31084950b 2024-09-14 17:09:51 +02:00 Compare
abian commented 2024-09-14 17:23:19 +02:00
Author
Member
Copy Link

Anything to do before we can merge this? (ping @Itms)

Anything to do before we can merge this? (ping @Itms)
Itms commented 2024-09-15 13:45:03 +02:00
Owner
Copy Link

If the reviewer is OK then it's good to go. @sera do you want to merge this as your first team action? πŸ˜„

If the reviewer is OK then it's good to go. @sera do you want to merge this as your first team action? πŸ˜„
sera merged commit b15eb6909e into main 2024-09-18 06:47:00 +02:00
abian deleted branch left-click-portrait 2024-09-18 12:02:09 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Stan
sera
Labels
Clear labels   
Closed
Duplicate

  
Closed
Fixed

  
Closed
Invalid

  
Closed
Needs info

  
Closed
Won't fix

  
Closed
Works for me

  
Difficulty
Hard

  
Difficulty
Medium

  
Difficulty
Simple

  
Needed for Beta

   
Needs Design Input

   
Needs Info

   
Pathfinding
  
Priority
1: Release Blocker

  
Priority
2: Must Have

  
Priority
3: Should Have

  
Priority
4: Nice To Have

  
Priority
5: If Time Permits

  
Regression
  
Theme
AI

  
Theme
Art & Animation

  
Theme
Atlas editor

  
Theme
Build & Packages

  
Theme
Core engine

  
Theme
Documentation

  
Theme
Internationalization & Localization

  
Theme
Maps

  
Theme
Multiplayer Lobby

  
Theme
Music & Sound FX

  
Theme
Network

  
Theme
Non-game systems

  
Theme
Simulation

  
Theme
UI & Simulation

  
Theme
UI – Game setup

  
Theme
UI – In-game

  
Theme
UI – Miscellaneous

  
Theme
Website & Forum

  
Type
Defect

  
Type
Enhancement

  
Type
Task

No Label
Closed
Duplicate
Closed
Fixed
Closed
Invalid
Closed
Needs info
Closed
Won't fix
Closed
Works for me
Difficulty
Hard
Difficulty
Medium
Difficulty
Simple
Needed for Beta
Needs Design Input
Needs Info
Pathfinding
Priority
1: Release Blocker
Priority
2: Must Have
Priority
3: Should Have
Priority
4: Nice To Have
Priority
5: If Time Permits
Regression
Theme
AI
Theme
Art & Animation
Theme
Atlas editor
Theme
Build & Packages
Theme
Core engine
Theme
Documentation
Theme
Internationalization & Localization
Theme
Maps
Theme
Multiplayer Lobby
Theme
Music & Sound FX
Theme
Network
Theme
Non-game systems
Theme
Simulation
Theme
UI & Simulation
Theme
UI – Game setup
Theme
UI – In-game
Theme
UI – Miscellaneous
Theme
Website & Forum
Type
Defect
Type
Enhancement
Type
Task
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: 0ad/0ad#7018
No description provided.
Delete Branch "abian/0ad:left-click-portrait"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?