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

EmbeddedCollectionEditor #33

Closed joeychu closed 9 years ago

joeychu commented 9 years ago

Hi!

I'm working on an app based on your sample FSM editor. I allow multiple triggers to be specified for a single transition, and each of these triggers is itself a struct of several properties.

I'm using the EmbeddedCollectionEditor to edit this list of multiple triggers. With ATF 3.9, the refresh of these UI elements is not working properly. If I select a transition, the property window switches to display that transition. The buttons are updated, as is the [n items] label, but the UI elements for each of the items in the list does not show up. If I collapse the category and expand it again, the elements will be displayed properly. It does show up the very first time I select anything, though.

This all worked properly with ATF 3.8.

Can you suggest a fix? Thanks.

Ron2 commented 9 years ago

Thank you very much for the bug report. Sorry you ran into this problem. One of us (Alan) is going to look into this. It's a little bit challenging because only our DOM Property Editor sample app uses EmbeddedCollectionEditor currently, and this sample app can't reproduce the problem. We will make use of EmbeddedCollectionEditor in another sample app, probably the FSM Editor, like you did.

--Ron

abeckus commented 9 years ago

I couldn't reproduce the problem. For test case, I changed TransitionType in FsmEditor to have list of triggers. EmbeddedCollectionEditor properly refreshes when changing selected transition.

What type of attributes your trigger has and what are the UITypeEditor you are defining for each attribute?

For your reference here is how I defined triggerType.

/xs:complexType Here are the relevant changes in SchemaLoader.cs 1- Add Descriptor to triggerType. Schema.triggerType.Type.SetTag( new PropertyDescriptorCollection( new PropertyDescriptor[] { new AttributePropertyDescriptor( "Label".Localize(), Schema.triggerType.labelAttribute, null, "Label displayed on trigger".Localize(), false), new AttributePropertyDescriptor( "Id".Localize(), Schema.triggerType.idAttribute, null, "Trigger id".Localize(), false), new AttributePropertyDescriptor( "Active".Localize(), Schema.triggerType.activeAttribute, null, "Is trigger active".Localize(), false, new BoolEditor()) ``` })); ``` 2- create new instance of EmbeddedCollectionEditor var collectionEditor = new EmbeddedCollectionEditor(); ``` // the following lambda's handles (add, remove, move ) items. collectionEditor.GetItemInsertersFunc = (context) => { var insertors = new EmbeddedCollectionEditor.ItemInserter[1]; var list = context.GetValue() as IList; if (list != null) { var childDescriptor = context.Descriptor as ChildPropertyDescriptor; if (childDescriptor != null) { insertors[0] = new EmbeddedCollectionEditor.ItemInserter(childDescriptor.ChildInfo.Type.Name, delegate { DomNode node = new DomNode(childDescriptor.ChildInfo.Type); if (node.Type.IdAttribute != null) { node.SetAttribute(node.Type.IdAttribute, node.Type.Name); } list.Add(node); return node; }); return insertors; } } return EmptyArray.Instance; }; collectionEditor.RemoveItemFunc = (context, item) => { var list = context.GetValue() as IList; if (list != null) list.Remove(item.Cast()); }; collectionEditor.MoveItemFunc = (context, item, delta) => { var list = context.GetValue() as IList; if (list != null) { DomNode node = item.Cast(); int index = list.IndexOf(node); int insertIndex = index + delta; if (insertIndex < 0 || insertIndex >= list.Count) return; list.RemoveAt(index); list.Insert(insertIndex, node); } }; ``` 3- Add ChildPropertyDescriptor transitionType Schema.transitionType.Type.SetTag( new PropertyDescriptorCollection( new PropertyDescriptor[] { new AttributePropertyDescriptor( "Label".Localize(), Schema.transitionType.labelAttribute, null, "Label displayed on transition".Localize(), false), new AttributePropertyDescriptor( "Action".Localize(), Schema.transitionType.actionAttribute, null, "Action performed when making transition".Localize(), false), new ChildPropertyDescriptor( "Trigger".Localize(), Schema.transitionType.triggerChild, null, "Triggers".Localize(), false, collectionEditor) })); Alan
joeychu commented 9 years ago

My triggerType is defined as follows:

    Schema.transitionTriggerType.Type.SetTag(
        new PropertyDescriptorCollection(
            new PropertyDescriptor[] {
                new AttributePropertyDescriptor(
                    Localizer.Localize("Script"),
                    Schema.transitionTriggerType.triggerAttribute,
                    null,
                    Localizer.Localize("Script which triggers transition"),
                    false),
                new AttributePropertyDescriptor(
                    Localizer.Localize("Params"),
                    Schema.transitionTriggerType.paramsAttribute,
                    null,
                    Localizer.Localize("Script parameters"),
                    false),
                new AttributePropertyDescriptor(
                    Localizer.Localize("Blend Time"),
                    Schema.transitionTriggerType.blendTimeAttribute,
                    null,
                    Localizer.Localize("Time to blend to destination state"),
                    false)
            }));

My EmbeddedCollectionEditor code is the exact same as yours. Copied it straight out of one of the samples.

This is my transitionType with the triggerType ChildPropertyDescriptor:

    Schema.transitionType.Type.SetTag(
        new PropertyDescriptorCollection(
            new PropertyDescriptor[] {
                new ChildPropertyDescriptor(
                    Localizer.Localize("Triggers"),
                    Schema.transitionType.transitionTriggerChild,
                    null,
                    Localizer.Localize("List of triggers"),
                    false,
                    collectionEditor),
                new AttributePropertyDescriptor(
                    Localizer.Localize("Name"),
                    Schema.transitionType.labelAttribute,
                    null,
                    Localizer.Localize("Transition Name"),
                    false),
                new AttributePropertyDescriptor(
                    Localizer.Localize("Priority"),
                    Schema.transitionType.priorityAttribute,
                    null,
                    Localizer.Localize("Transition Priority"),
                    false,
                    new NumericEditor(typeof(int)))
            }));
joeychu commented 9 years ago

I use the same EmbeddedCollectionEditor instance for a variety of properties in multiple descriptors, but I tried giving a unique one to the triggerType and it didn't help. (All of my cases produce the same non-refreshing behavior, not just my triggerType.)

For reference, here is how it appears unrefreshed: unrefreshed

And here it is after I have collapsed the category and re-expanded it: refreshed

abeckus commented 9 years ago

Still unable to reproduce the bug in our local branch. So I pushed the FsmEditor changes to master branch. Please pull the latest and run FsmEditor to see if you can reproduce the problem. If so, please provide more detailed reproduction steps If not, please compare your local changes with FsmEditor to see what could cause this issue. Because it was working in ATF3.8, I think the bug is in ATF but for some reason my test case doesn't hit the code path that cause this refresh bug.

Thanks Alan

joeychu commented 9 years ago

Thanks!

I'm not sure what's happening here, but this is what I'm seeing when I run your new version: sample_collection

It's slightly different from the embedded editor that I see in my FSM (see screenshots from earlier post). My "Triggers" label is on the same line as the buttons, and collection editor seems to be entirely in the right column. In yours, the collection editor seems to be inline with the rest of your properties, with the label/id/active in the left column and your values in the right column.

I'm not sure what is different about the declarations, but I am able to reproduce the behavior in the sample. I added a new property to the state that also uses a collection editor, and this one displays the same way mine does, and reproduces the buggy behavior.

I added this to the FSM.xsd:

  <xs:complexType name="substateType">
    <xs:attribute name="label" type="xs:string" default="substate"/>
    <xs:attribute name="id" type="xs:int" default="0"/>
    <xs:attribute name="someField" type="xs:string" use="optional"/>
  </xs:complexType>

This becomes the stateType in FSM_customized.xsd:

    <xs:complexType name="stateType">
      <xs:complexContent>
        <xs:extension base="stateType">
          <xs:sequence>
            <xs:element name="substates" type="substateType" minOccurs="0" maxOccurs="unbounded"/>
          </xs:sequence>
          <xs:attribute name="entryAction" type="xs:string"  use="optional" />
          <xs:attribute name="action" type="xs:string"  use="optional" />
          <xs:attribute name="exitAction" type="xs:string"  use="optional" />
        </xs:extension>
      </xs:complexContent>
    </xs:complexType>

This was added to the SchemaLoader:

                Schema.substateType.Type.SetTag(
                    new PropertyDescriptorCollection(
                        new PropertyDescriptor[] {
                            new AttributePropertyDescriptor(
                                "Label".Localize(),
                                Schema.substateType.labelAttribute,
                                null,
                                "Label displayed by the substate".Localize(),
                                false),
                            new AttributePropertyDescriptor(
                                "Id".Localize(),
                                Schema.substateType.idAttribute,
                                null,
                                "Substate id".Localize(),
                                false),
                            new AttributePropertyDescriptor(
                                "SomeField".Localize(),
                                Schema.substateType.someFieldAttribute,
                                null,
                                "Some field".Localize(),
                                false)
                } ) );

And the collectionEditor added to the state here:

                Schema.stateType.Type.SetTag(
                    new PropertyDescriptorCollection(
                        new PropertyDescriptor[] {
                            new AttributePropertyDescriptor(
                                "Name".Localize(),
                                Schema.stateType.labelAttribute, // 'nameAttribute' is unique id, label is user visible name
                                null,
                                "State name".Localize(),
                                false),
                            new AttributePropertyDescriptor(
                                "Size".Localize(),
                                Schema.stateType.sizeAttribute,
                                null,
                                "State size".Localize(),
                                false),
                            new AttributePropertyDescriptor(
                                "Hidden".Localize(),
                                Schema.stateType.hiddenAttribute,
                                null,
                                "Whether or not state is hidden".Localize(),
                                false,
                                new BoolEditor()),
                            new AttributePropertyDescriptor(
                                "Start".Localize(),
                                Schema.stateType.startAttribute,
                                null,
                                "Whether or not state is the start state".Localize(),
                                false,
                                new BoolEditor()),
                            new AttributePropertyDescriptor(
                                "Entry Action".Localize(),
                                Schema.stateType.entryActionAttribute,
                                null,
                                "Action performed when entering state".Localize(),
                                false),
                            new AttributePropertyDescriptor(
                                "Action".Localize(),
                                Schema.stateType.actionAttribute,
                                null,
                                "Action performed while in state".Localize(),
                                false),
                            new AttributePropertyDescriptor(
                                "Exit Action".Localize(),
                                Schema.stateType.exitActionAttribute,
                                null,
                                "Action performed when exiting state".Localize(),
                                false),
                            new ChildPropertyDescriptor(
                                "Substates".Localize(),
                                Schema.stateType.substatesChild,
                                null,
                                "List of substates".Localize(),
                                false,
                                collectionEditor)
                    } ) );

Apologies for all of the cut&paste code, want to make sure I don't leave out anything potentially important.

Also had to move the declaration of the collectionEditor higher up in the function so it would compile.

I tried to regen the Schema.cs using GenSchemaDef.bat, but for some reason it was crashing for me even when I removed all of my changes to the xsds, so I ended up hand-editing the changes into Schema.cs.

I tried this both making all of these additions, and also removing the existing collectionEditor from the trigger in case it had to do with multiple uses of the collectionEditor. It didn't make a difference. The collectionEditor is entirely in the right-hand property column and exhibits the bad refresh behavior.

abeckus commented 9 years ago

Now I can reproduce the bug, thanks for posting the code snippets I will update you as soon as I have a fix.

Regarding label location and interlining EmbeddedCollectionEditor It is a feature to save horizontal space. You can disable it by removing the following code in SchemaLoader.cs

region IInitializable Members

    void IInitializable.Initialize()
    {

        PropertyGridView propertyGridView = m_propertyEditor.PropertyGrid.PropertyGridView;
        if (propertyGridView.CustomizeAttributes != null)
            throw new InvalidOperationException("Someone else set PropertyGridView's CustomizeAttributes already");
        propertyGridView.CustomizeAttributes = new[]
            {
                new PropertyView.CustomizeAttribute("Triggers".Localize(), horizontalEditorOffset:0, nameHasWholeRow:true),                    
            };
    }
    #endregion

GenSchemaDef.bat crashing: Please make sure that DomGen.exe exist at ATF\DevTools\DomGen\bin if DomGen.exe exist then please send me the error message.

Alan

abeckus commented 9 years ago

Fixed, Please pull the latest changes and verify that the fix works for you. Thank you again for reporting the issue and for providing code to reproduce it. Please let us know if you find any other issue.

Thanks Alan

joeychu commented 9 years ago

Yup, that fixed the problem! Thanks!

re:domgen crashing for me - it may be something local. If it's not, I'll open a separate issue.