LogicalError / realtime-CSG-for-unity

Realtime-CSG, CSG level editor for Unity
https://realtimecsg.com
MIT License
735 stars 77 forks source link

When editing a model that's inside a prefab, the tool adds new brushes to the open scene instead of the prefab #321

Open robclouth opened 3 years ago

robclouth commented 3 years ago

When editing a model that's inside a prefab, the tool adds new brushes to the open scene instead of the prefab. It seems to "remember" to add to the prefab if you edit a brush then go back to generate, but then it will forget it again and you have to do the edit trick again. Any ideas?

alexjhetherington commented 2 years ago

Unable to reproduce in 2019 LTS, 2021 LTS.

@robclouth please could you provide the version you are on + video or project with replication.

Otherwise I recommend this issue is closed.

Mark-LaCroix commented 2 years ago

I'm having the same issue. Here's a video of it "in action" :

https://user-images.githubusercontent.com/1129348/177970437-a7f8eb32-66f5-467f-bd07-c725c323a44d.mp4

I'm using Unity 2021.3.3 and Realtime CSG 1.57.2 via Package Manager git URL.

robclouth commented 2 years ago

That's exactly it

Mark-LaCroix commented 2 years ago

I've traced the problem to Generator.Base.GenerateBrushObjects() where lastUsedModelTransform (which gets its value from SelectionUtility.LastUsedModel) is somehow null when you're editing a prefab (or really, when a Model object is part of a prefab, as this issue also occurs outside of prefab editing mode). Thus, a new "Model" GameObject is created for the brush to go into.

I'm not yet sure how SelectionUtility.LastUsedModel is normally set, but I'll keep looking.

Mark-LaCroix commented 2 years ago

Once I comment out this block inside of SelectionUtility.LastUsedModel.set...

// don't want new stuff to be added to a prefab instance
if (CSGPrefabUtility.IsPrefabAssetOrInstance(model.gameObject))
{
    continue;
}

...adding brushes now work as expected in prefab isolation mode, and also works just fine in scene mode, as long as there isn't another (non-prefab) Model object, in which case it seems to default to that with no problem.

So, fixed? But... the comment in this code block implies that there's some reason we actually wouldn't want to do this. I can't find any downsides, but I've only been testing it for a few minutes. 😂

I can do a PR for this, assuming it doesn't brake something else that I'm ignorant to. Let me know.

nukeandbeans commented 1 year ago

Once I comment out this block inside of SelectionUtility.LastUsedModel.set...

So, fixed? But... the comment in this code block implies that there's some reason we actually wouldn't want to do this. I can't find any downsides, but I've only been testing it for a few minutes. 😂

I can do a PR for this, assuming it doesn't brake something else that I'm ignorant to. Let me know.

Out of curiosity, how is it working now? Any side effects?

You're more than welcome to submit a PR to this branch of my own fork, so it can be included in #349

silentorb commented 1 year ago

Commenting out either or both of the CSGPrefabUtility.IsPrefabAssetOrInstance checks isn't fixing the bug for me in 2022.1.10.

As a workaround, if I first add an empty CSG Model to the prefab and then start generating brushes, the brushes are created within the prefab instead of the scene.

In case the root of the issue does involve CSGPrefabUtility.IsPrefabAssetOrInstance, just to clear up some confusion: there are two uses of it in SelectionUtility. @Mark-LaCroix mentions set but pasted code for the get.