SonyWWS / ATF

Authoring Tools Framework (ATF) is a set of C#/.NET components for making tools on Windows. ATF has been in continuous development in Sony Computer Entertainment's (SCE) Worldwide Studios central tools group since early 2005. ATF has been used by most SCE first party studios to make many custom tools such as Naughty Dog’s level editor and shader editor for The Last of Us, Guerrilla Games’ sequence editor for Killzone games (including the Killzone: Shadow Fall PS4 launch title), an animation blending tool at Santa Monica Studio, a level editor at Bend Studio, a visual state machine editor for Quantic Dream, sound editing tools, and many others.
Apache License 2.0
1.89k stars 262 forks source link

PropertyGridView's Pick() could return wrong object #54

Closed Ron2 closed 8 years ago

Ron2 commented 8 years ago

When multiple child collection editors are used in the Property Editor (and so there are multiple child PropertyGridView objects), this Pick method can return the wrong item. This was happening because FindChildControls() returns the first instance of a PropertyGridView, but there’s no ordering from top-to-bottom and the PropertyGridView may not even be visible.

Ron2 commented 8 years ago

I can't quite reproduce a problem in the DOM Property Editor sample app, but I can get close, and I hope you can see that there could be a problem. If you incorporate this change, and put a breakpoint on the "return null" line, then you can easily get this breakpoint to go off, but in practice, in my testing, Pick() would still have returned null in DOM Property Editor. In our version of LevelEditor, we have many child collection editors and, unlike DOM Property Editor, we can select different objects, so it was easy to get this Pick() method to return an item from the wrong PropertyGridView, and so context menu commands would act on the wrong object.

abeckus commented 8 years ago

Thanks I merged the changes locally and it will be up for next ATF update. Yes, I think it would be better to make DOM Property Editor to expose list of objects instead of binding property editor to one hidden object. This way more code paths will be tested. I will try to make time to have it changed for next update.

Alan

Ron2 commented 8 years ago

Thanks, Alan!