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

Expose GridMapEditorPlugin to scripts and add methods to manipulate to the selection and selected palette item #99639

Merged

Conversation

badsectoracula
Copy link
Contributor

@badsectoracula badsectoracula commented Nov 24, 2024

This PR allows editor scripts and plugins to access GridMapEditorPlugin and adds methods for extending the editor's functionality by allowing manipulation of the selection and selected palette item.

This addresses and closes godotengine/godot-proposals#11161. It can also help with custom grid map tools that could be added later by scripts as i wrote in godotengine/godot-proposals#11206 (comment).

The video below shows how it could be used to implement a script that replaces tiles in the selection:

Godot.Gridmap.Selection.Plugin.webm

(obviously this is just an example -and probably the replace functionality should be core part of the editor- to show how the API can be used but it could also be used for making game-specific tools like assigning data -to be stored elsewhere, like in a node- to cells, or to pre-fill a gridmap with some procedural code that can be then manually edited, or to quickly set data and/or colors -especially useful for data as they can be something different than color- if #94282 is merged, etc)

EDIT:

Attached MRP with the script in the screenshot (though different map). To test it open the "node_3d.tscn" scene, select the "Floor" gridmap, pick "Floor3" from the available tiles (the cracked one), activate the "Replace Tile" dock (should be at bottom left), click the "Pick original tile" button (the label should change to "Select an item to replace 4 with"), make a rectangular selection in the viewport with some cracked tiles inside, pick "Floor2" from the available tiles (the one with the dark smudge) and click the "Replace in selection" button (note: if some tiles at the selection edges do not change that is because the tiles have their origin at the corner and thus the selection can be misleading a bit - that is an issue with the assets, not this PR).

The MRP:
gridmap-editor-scripting-test.zip

@badsectoracula badsectoracula requested review from a team as code owners November 24, 2024 17:48
@badsectoracula badsectoracula force-pushed the get-selected-gridmap-cells branch 2 times, most recently from 6589c81 to ea8cf77 Compare November 24, 2024 17:54
Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surface-level look at the docs

modules/gridmap/doc_classes/GridMapEditorPlugin.xml Outdated Show resolved Hide resolved
modules/gridmap/doc_classes/GridMapEditorPlugin.xml Outdated Show resolved Hide resolved
modules/gridmap/doc_classes/GridMapEditorPlugin.xml Outdated Show resolved Hide resolved
modules/gridmap/doc_classes/GridMapEditorPlugin.xml Outdated Show resolved Hide resolved
@Mickeon Mickeon added this to the 4.x milestone Nov 24, 2024
@badsectoracula badsectoracula force-pushed the get-selected-gridmap-cells branch 2 times, most recently from 8f2d311 to 1fa8a5f Compare November 24, 2024 20:16
@badsectoracula badsectoracula requested a review from a team as a code owner November 24, 2024 20:16
@badsectoracula badsectoracula force-pushed the get-selected-gridmap-cells branch from 1fa8a5f to 148fb77 Compare November 24, 2024 20:19
@badsectoracula
Copy link
Contributor Author

Latest PR fixed CI and made the modifications to the docs and API @Mickeon suggested and fixed the issues stemmed from multiple instantiations of the GridMapEditorPlugin class during its registration that i didn't notice. Instead of doing it the constructor, the UI control is now created when the plugin is attached to the tree (and destroyed when it is detached), which i think is a better place to create it anyway.

@Calinou
Copy link
Member

Calinou commented Nov 29, 2024

@badsectoracula Can you upload the script you used for testing somewhere (or ideally a full MRP)? This will make it easier to review the PR and document its functionality 🙂

@badsectoracula
Copy link
Contributor Author

@Calinou i made and attached an MRP. It is the same script though i used some different assets i made myself.

@coppolaemilio coppolaemilio requested a review from Calinou December 3, 2024 16:15
Copy link
Member

@hpvb hpvb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the doc changes were approved, the code changes look obviously correct to me and desirable. Lets gooooo.

@hpvb hpvb modified the milestones: 4.x, 4.4 Dec 16, 2024
Copy link
Contributor

@tetrapod00 tetrapod00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving for docs only

modules/gridmap/doc_classes/GridMapEditorPlugin.xml Outdated Show resolved Hide resolved
modules/gridmap/doc_classes/GridMapEditorPlugin.xml Outdated Show resolved Hide resolved
modules/gridmap/doc_classes/GridMapEditorPlugin.xml Outdated Show resolved Hide resolved
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, it works as expected. Code looks good to me.

Note that this is the first time we expose a SomethingEditorPlugin to the scripting API:

image

Given this is too specific to expose within EditorInterface (and it's part of a module), I can't think of a better solution to this. @KoBeWi @Zylann What do you think?

@badsectoracula badsectoracula force-pushed the get-selected-gridmap-cells branch from 148fb77 to baf03e4 Compare December 16, 2024 19:31
@badsectoracula
Copy link
Contributor Author

Given this is too specific to expose within EditorInterface (and it's part of a module), I can't think of a better solution to this. @KoBeWi @Zylann What do you think?

You mean the way the instance is accessed (by obtaining a node via the editor interface and then looking for the plugin node)? While perhaps a cleaner convenience API could probably be exposed via EditorInterface, since all plugins seem to end up (AFAICT anyway) as nodes, if nothing else it can be a useful way to demonstrate how they could communicate with each other (which was a concern i had while working on this - i.e. how plugins could exchange data / expose objects to each other).

@akien-mga akien-mga merged commit 076d126 into godotengine:master Dec 17, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@badsectoracula badsectoracula deleted the get-selected-gridmap-cells branch December 17, 2024 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a method to access the GridMap editor's current selection
6 participants