Skip to content

Add multiselect to asset dock - #739

Open
aidandavey wants to merge 2 commits into
TokisanGames:mainfrom
aidandavey:AssetDock-multi-select
Open

Add multiselect to asset dock#739
aidandavey wants to merge 2 commits into
TokisanGames:mainfrom
aidandavey:AssetDock-multi-select

Conversation

@aidandavey

Copy link
Copy Markdown
Contributor

Allows selection of multiple mesh assets in the dock for instancing or removal.

Always keeps at least one asset selected.

Click tile to select/deselect

image

@Xtarsia

Xtarsia commented Jun 25, 2025

Copy link
Copy Markdown
Collaborator

Theres a few issues testing this:

Having more than 1 mesh asset, and then trying to remove the last asset when not currently selected, forces it to be selected. Which then causes errors when painting assets.

Having CrystalC as asset ID 0, Rock A as ID 1, have both selected, then add a 3rd asset, Rock B, causes ID 0 to render as Rock B, along with the thumbnail to swap to Rock B, even though CrystalC is still the chosen Scene file in the inspector.
Saving and reloading didnt restore the rendered meshes to the correct state, even though the Scene file for ID0 was still showing as CrystalC.
Re-loading CrystalC for ID0 results in ID2 becoming CrystalC.
This issue seemed to only get worse as the number of meshes inceased.

When it did work, it was nice to use, I can see it being very useful!

@aidandavey
aidandavey force-pushed the AssetDock-multi-select branch 2 times, most recently from e02f0f7 to ceed9fc Compare June 26, 2025 21:29
@TokisanGames TokisanGames added this to the 1.1 milestone Jun 27, 2025
@TokisanGames TokisanGames added the enhancement New feature or request label Jun 27, 2025
@aidandavey
aidandavey force-pushed the AssetDock-multi-select branch 2 times, most recently from b613a8a to 6297904 Compare June 28, 2025 20:56
@aidandavey

Copy link
Copy Markdown
Contributor Author

Theres a few issues testing this:

Having more than 1 mesh asset, and then trying to remove the last asset when not currently selected, forces it to be selected. Which then causes errors when painting assets.

Having CrystalC as asset ID 0, Rock A as ID 1, have both selected, then add a 3rd asset, Rock B, causes ID 0 to render as Rock B, along with the thumbnail to swap to Rock B, even though CrystalC is still the chosen Scene file in the inspector. Saving and reloading didnt restore the rendered meshes to the correct state, even though the Scene file for ID0 was still showing as CrystalC. Re-loading CrystalC for ID0 results in ID2 becoming CrystalC. This issue seemed to only get worse as the number of meshes inceased.

When it did work, it was nice to use, I can see it being very useful!

Both of these issues have now been addressed, thanks!

@aidandavey
aidandavey force-pushed the AssetDock-multi-select branch from f927c92 to a9ce529 Compare June 28, 2025 21:05
@TokisanGames
TokisanGames marked this pull request as draft November 19, 2025 12:00
@TokisanGames TokisanGames moved this to In Progress in Terrain3D Roadmap Dec 6, 2025
@TokisanGames TokisanGames moved this from In Progress to 1.2 in Terrain3D Roadmap Dec 7, 2025
@TokisanGames TokisanGames modified the milestones: 1.1, 1.2 Dec 7, 2025
@TokisanGames TokisanGames moved this from 1.2 to 1.1 in Terrain3D Roadmap May 27, 2026
@TokisanGames TokisanGames modified the milestones: 1.2, 1.1 May 27, 2026
@TokisanGames TokisanGames moved this from 1.1 to In Progress in Terrain3D Roadmap May 27, 2026
@aidandavey
aidandavey force-pushed the AssetDock-multi-select branch from a9ce529 to a7a6940 Compare June 29, 2026 20:22
@aidandavey

Copy link
Copy Markdown
Contributor Author

This had gone through a few changes since the last tests to bring it up to date and to add support for 4.6


This adds the ability to select multiple Mesh Assets and paint them in a single operation. I changed the selected_id int to an Array

Left clicking a tile in the asset dock will add it to the selection, or deselect it if it is already selected.

Right clicking a tile will add it to the selection, but will not remove from the selection.

Density is independent for each MA, so you can paint grass at high density with lower density trees.

Instance removal is similarly multi-select, it will remove instances of the MAs that are currently selected.

Search - because we are selectively showing tiles here, we get a mismatch between the tile id and the asset id. To fix it, I have changed to a system whereby the selected_ids array contains the asset_ids, rather than tile ids. This means we can make a selection, perform a search, add a new MA to the selection, clear the search without losing our original selection.

@aidandavey
aidandavey marked this pull request as ready for review June 29, 2026 20:26

@TokisanGames TokisanGames left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Here's a first pass reviewing only the C++. So far so good, only one crash and some minor things.

Comment thread src/terrain_3d_instancer.h Outdated
Comment thread src/terrain_3d_instancer.h Outdated
Comment thread src/terrain_3d_instancer.h Outdated
Comment thread src/terrain_3d_editor.cpp

_brush_data["enable_texture"] = p_data.get("enable_texture", true);
_brush_data["texture_filter"] = p_data.get("texture_filter", false);
_brush_data["asset_id"] = CLAMP(int(p_data.get("asset_id", 0)), 0, ((_tool == INSTANCER) ? Terrain3DAssets::MAX_MESHES : Terrain3DAssets::MAX_TEXTURES) - 1);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This defaulted to id 0. Now texture painting is ignored if there is no asset_ids. Perhaps we should insert [0] if it doesn't exist.

Also it doesn't sanitize the brush data. Sanitization was limited only to 0-32/256 before instead of the actual count. We could check _terrain->get_assets()->get_texture_count() and _terrain->get_assets()->get_mesh_count().

@aidandavey aidandavey Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have implemented sanitation and need to test it

Comment thread src/terrain_3d_instancer.h Outdated
Comment thread src/terrain_3d_instancer.cpp Outdated
int mesh_id = p_params.get("asset_id", 0);
if (p_params.has("asset_id")) {
LOG(ERROR, "We are no longer using asset_id, we need an array of asset_ids");
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This can be removed and instead it can be documented here and here and here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed the code and updated the docs (edited the xml files)
When it's merged could you update the tracker issue please? Or I can add it as a new comment.

real_t random_tilt = CLAMP(real_t(p_params.get("random_tilt", 10.f)), 0.f, 180.f); // degrees
bool align_to_normal = bool(p_params.get("align_to_normal", false));
for (int i = 0; i < mesh_ids.size(); i++) {
int mesh_id = mesh_ids[i];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can be for (const int mesh_id : mesh_ids)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We need the current index from the selected_index array when we call _update_density_count(index, p_density)

Comment thread src/terrain_3d_instancer.cpp Outdated
Comment thread src/terrain_3d_instancer.h Outdated
Comment thread src/terrain_3d_instancer.h
@aidandavey
aidandavey force-pushed the AssetDock-multi-select branch from a7a6940 to 8ee87f1 Compare July 29, 2026 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request usability UI

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants