Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New extension SelectionTools. #974

Merged
merged 4 commits into from
Aug 28, 2023
Merged

New extension SelectionTools. #974

merged 4 commits into from
Aug 28, 2023

Conversation

ayushbhardwas
Copy link
Contributor

The extension provides functionality to draw different types of selections in order to select object instances present on the scene.

How to use:

  • Create a shape painter object. It would be used to draw the selection on the screen.
  • Add the Setup selection tools action to an event.
  • Add the setup action for the required selection. Choose from: Set rectangular selection, Set polygonal selection, and Set lasso selection.

Example

@ayushbhardwas ayushbhardwas requested a review from a team as a code owner August 14, 2023 09:17
@ayushbhardwas
Copy link
Contributor Author

It seems I am not allowed to access gdjs.evtTools.input and runtimeScene.getObjects(). I can maybe do a workaround for input. But I am not quite sure what to do about the getObjects() function, it is being used to get access to the shape painter object to draw the selection. The only other way I can think of is if I make this into a behavior so that I have access to the shape painter object at all times. This is an option except I was trying to decouple the selection feature from the shape painter object, to save the user from the hassle of having to pass the object in all the actions/conditions. But maybe I am wrong in thinking that and maybe the right move is to make it a behavior. I'd appreciate your feedback and/or advice.

@4ian
Copy link
Collaborator

4ian commented Aug 14, 2023

Hi @ayushbhardwas - this looks nice! :)

  1. For gdjs.evtTools.input, it's indeed best to avoid it. Can you use InputManager instead (https://docs.gdevelop.io/GDJS%20Runtime%20Documentation/classes/gdjs.RuntimeGame.html#getInputManager). Get it through getGame() ont the runtimeScene
  2. For runtimeScene.getObjects, as you mentioned it's usually I think a good idea to make a behavior because it means you will have access to the shape painter. And for the objects to select/unselect, then usually they are passed as parameter of the actions/conditions.

I think it would be nice to try to fix both of these. I understand it's a fair amount of rework to convert this to a behavior, but please give a try and give a try to avoiding getObjects too :)

If you struggle too much, we'll add exceptions so we can merge your extension... but I think it would be future proof to fix both of these 👍

Thank you again for this nice work! 👍

@ayushbhardwas
Copy link
Contributor Author

Thanks a lot, @4ian
I'm incredibly grateful that you liked it and I will surely work on fixing the issues. 😊

@ayushbhardwas
Copy link
Contributor Author

ayushbhardwas commented Aug 14, 2023

Changes made:

  • Added Selection painter behavior.
  • Migrated events related to shape painter to the new behavior eliminating the need to use runtimeScene.getObjects.
  • Removed the Setup selection tools action. Since the painter is now identified via the behavior thus eliminating the need for this action.
  • Rewrote the shape drawing logic using events (earlier js) since now, the shape painter object is not being accessed through runtimeScene. Thus also making the extension more hacker/tinkerer friendly.
  • The previous change also eliminates the need for gdjs.evtTools.input.

How to use:

  • Activate SelectionTools using the De/Activate selection tools action.
  • Create a shape painter object and add the Selection painter behavior to it. Create a single instance of this object in the scene.
  • Set the type of selection using one of these actions: Set rectangular selection, Set polygonal selection, and Set lasso selection.

Example

@tristanbob
Copy link
Contributor

I haven't looked at the extension, but in your example game, units are not highlighted until the release happens.
Is there a "preselected" state that can be used to determine which objects would be selected when the mouse releases?
(RTS Unit Selection provides this)

@ayushbhardwas
Copy link
Contributor Author

ayushbhardwas commented Aug 14, 2023

@tristanbob No, as you speculated, there isn't a "preselected" state. I'm not sure why you'd want that, maybe you can help me understand that better.

The extension does not use object variables as flags, it just filters the current context. Furthermore, it's slightly expensive to run the checks for polygons (lasso and polygonal selection) each frame, depending on how big the polygon is and how many instances of the object(s) we wish to select are present in the scene.

Edit: My bad, the current condition is already doing the check each cycle so there's no reason why we can not do that 😅. And we can simply have a preselect bool parameter that can be turned on or off even if we don't use an object variable, it will be the same as the normal select condition but won't wait for the shape to be completed as you suggested. Thanks, Tristan ❤️ I think I can do that.

I remember our conversation about merging this extension with RTS Unit Selection, which when I looked into it is a very different sort of tool. I think it adds a lot that may be useful for a certain type of game I just don't see myself using it that often. This is supposed to be a more barebone extension. But I haven't forgotten about it, I think RTS Unit Selection would benefit immensely by having a lasso select, which if you allow I would definitely want to implement. On a similar note, you'd agree that you never really use polygonal selection to select units in an RTS so I don't think it needs that, not that it can not be implemented, just that it's not commonly seen in RTS games.

@ayushbhardwas
Copy link
Contributor Author

Changes made:

  • Added the ability to Pre select i.e. select instances before closing the shape. Activate or deactivate it using the Pre select boolean parameter in the Select condition.

Example

@ayushbhardwas
Copy link
Contributor Author

ayushbhardwas commented Aug 16, 2023

Changes made:

  • Added the ability to choose if the polygonal selection is represented by a True polygon. If the selection is a True polygon, dragging while the mouse button or touch is down will not place new vertices. This is especially useful for devices with a touch screen where there is no distinction between hover and pressed state, making it challenging to preselect without dragging the touch around which will also place new vertices. Something that you might not want. With True polygon the vertices get placed when the touch or mouse button is released making it possible to drag across a touch screen and preselect object instances.
  • Bug fixes
  • Changes to the description and condition names
  • Refactored pre-selection events to eliminate redundant checks.

[Edited] Notes:

  • Tue polygon property should probably be renamed to something like Discrete.
  • The extension still isn't ready for touch devices, I haven't tested it much. The polygonal selection might still face issues since the absence of cursor position is an undefined behavior. It's a simple fix but I think it's important that I file away any such minor issue to be fixed later once the core extension is reviewed by the team.

Example

@ayushbhardwas
Copy link
Contributor Author

I intend to make no further changes to the extension before the merger unless someone reports an issue on this PR. Consequently, I eagerly await the extension team's review and approval.

@4ian
Copy link
Collaborator

4ian commented Aug 28, 2023

@ayushbhardwas Many thanks again for the work on this and sorry for the delay here. I think it's fine to go ahead and quite flexible already! If you have a way to ensure this work on touch screen that would be perfect. In the meantime, because it's already well made and you've spent time on this, I'm merging this.

Many thanks and keep up the great work 👍

@4ian 4ian merged commit 526578b into GDevelopApp:main Aug 28, 2023
1 check passed
@ayushbhardwas
Copy link
Contributor Author

Thanks, Florian 😊
I waited in case someone found an issue or had ideas about changing something. I will work on an update that fixes things with touch controls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants