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

Circular reference detection is too aggressive #11

Closed
juh9870 opened this issue Sep 29, 2022 · 5 comments
Closed

Circular reference detection is too aggressive #11

juh9870 opened this issue Sep 29, 2022 · 5 comments

Comments

@juh9870
Copy link
Contributor

juh9870 commented Sep 29, 2022

I was trying to make a simple boolean nodes calculator, but quickly ran into "Circular reference detected"

Code
    [RequireComponent(typeof(RosettaUIRoot))]
    public class RosettaUIExample : MonoBehaviour
    {
        [Serializable]
        public enum RequirementType
        {
            AlwaysTrue,
            And,
            Or,
            Not,
            Checkbox,
        }

        [Serializable]
        public abstract class Requirement : IElementCreator
        {
            public abstract bool CalculateOutcome();
            public bool Ignored = false;
            public abstract Element CreateElement(LabelElement label);
        }

        [Serializable]
        public class AlwaysTrue : Requirement
        {
            public override bool CalculateOutcome() => true;
            public override Element CreateElement(LabelElement label) => UI.Column();
        }

        [Serializable]
        public class And : Requirement
        {
            public RequirementNode[] Operands = Array.Empty<RequirementNode>();
            public override bool CalculateOutcome() => Operands.All(e => e.req.CalculateOutcome());
            public override Element CreateElement(LabelElement label) => UI.Field(() => Operands);
        }

        [Serializable]
        public class Or : Requirement
        {
            public RequirementNode[] Operands = Array.Empty<RequirementNode>();
            public override bool CalculateOutcome() => Operands.Any(e => e.req.CalculateOutcome());
            public override Element CreateElement(LabelElement label) => UI.Field(() => Operands);
        }

        [Serializable]
        public class Not : Requirement
        {
            public RequirementNode Operand = new RequirementNode();
            public override bool CalculateOutcome() => !Operand.req.CalculateOutcome();
            public override Element CreateElement(LabelElement label) => UI.Field(() => Operand);
        }

        [Serializable]
        public class Checkbox : Requirement
        {
            public bool Active;
            public override bool CalculateOutcome() => Active;
            public override Element CreateElement(LabelElement label) => UI.Field(() => Active);
        }

        public static class RequirementFactory
        {
            public static Requirement Create(RequirementType type)
            {
                return type switch
                {
                    RequirementType.AlwaysTrue => new AlwaysTrue(),
                    RequirementType.And => new And(),
                    RequirementType.Or => new Or(),
                    RequirementType.Not => new Not(),
                    RequirementType.Checkbox => new Checkbox(),
                    _ => throw new ArgumentOutOfRangeException(nameof(type), type, null)
                };
            }
        }

        [Serializable]
        public class RequirementNode : IElementCreator
        {
            private RequirementType _requirementType;

            public RequirementType RequirementType
            {
                get => _requirementType;
                set
                {
                    if (_requirementType == value) return;
                    _requirementType = value;
                    req = RequirementFactory.Create(value);
                }
            }

            public Requirement req = new AlwaysTrue();

            public Element CreateElement(LabelElement label)
            {
                return UI.Column(
                    UI.Field(() => RequirementType),
                    UI.DynamicElementOnStatusChanged(() => RequirementType, _ => req.CreateElement(null))
                );
            }
        }

        public RequirementNode Req = new();
        private RosettaUIRoot _root;

        private void Start()
        {
            _root = GetComponent<RosettaUIRoot>();
            _root.Build(CreateElement());
        }

        private Element CreateElement()
        {
            return UI.Window(
                // ExampleTypes.Select(type => UI.WindowLauncher(type))
                UI.Field(() => Req),
                UI.Field("Outcome", () => Req.req.CalculateOutcome())
            );
        }
    }

This is the result I get in the UI
image
As you can see, all nodes are working normally, except for Not node that throws circular reference, even tho I clearly don't have any in my code.

Also, I have an unrelated question: Is there a better way to make an editor for this kind of data structure? Aka one "holder" that have a type field, and a body field that contains different classes depending on type. My current implementation requires to manually write UI controls for every possibly body class, and I'm concerned about performance implication of dozens of DynamicElements performing checks on type field every tick

@fuqunaga
Copy link
Owner

fuqunaga commented Oct 2, 2022

Sorry, the error message wasn't good.
RosettaUI checks for "type" cycles, so in this case it is because you tried to generate another RequirementNode UI within the RequirementNode UI.

Is there a better way to make an editor for this kind of data structure?

I feel this is the only way.
While not a radical solution, the amount of UI controls to write in this case can be reduced a bit with C# techniques.

e.g.

 public abstract class RequirementWithOperands: Requirement
{
    public RequirementNode[] Operands = Array.Empty<RequirementNode>();
    public override Element CreateElement(LabelElement label) => UI.Field(() => Operands);
}

[Serializable]
public class And : RequirementWithOperands
{
    public override bool CalculateOutcome() => Operands.All(e => e.req.CalculateOutcome());
}

...

@juh9870
Copy link
Contributor Author

juh9870 commented Oct 2, 2022

Sadly, this won't work because as you can See a bit down there, not all types use Operands, they can use different parameters

@fuqunaga
Copy link
Owner

fuqunaga commented Oct 2, 2022

You're right.
I was thinking of it as an example of slightly reducing the code description of types that use Operands.

My current implementation requires to manually write UI controls for every possibly body class

I think it has to be that way with a UI that uses different parameters.
You may be able to reduce the amount of code by using meta-programming such as reflection, but this is a pure C# problem and not specific to RosettaUI😅

@juh9870
Copy link
Contributor Author

juh9870 commented Oct 2, 2022

Maybe then add an option to disable circular reference detection? I will use code generation to automatically generate UI code, but I still need a way to avoid having circular reference error popping up in wrong places. Maybe switch type-based detection to instance-based detection?

@fuqunaga
Copy link
Owner

fuqunaga commented Oct 3, 2022

Yeah, now that I know about this case, I'll think about it.

github-actions bot pushed a commit that referenced this issue Nov 22, 2022
# [ga.fuquna.rosettaui-v1.0.1](ga.fuquna.rosettaui-v1.0.0...ga.fuquna.rosettaui-v1.0.1) (2022-11-22)

### Bug Fixes

* circular reference checks from type-based to reference-based [#11](#11) ([479bd7e](479bd7e))
* List always pass through circular reference detection [#12](#12) ([991c90a](991c90a))
github-actions bot pushed a commit that referenced this issue Nov 22, 2022
# [ga.fuquna.rosettaui-v1.0.1](ga.fuquna.rosettaui-v1.0.0...ga.fuquna.rosettaui-v1.0.1) (2022-11-22)

### Bug Fixes

* circular reference checks from type-based to reference-based [#11](#11) ([479bd7e](479bd7e))
* List always pass through circular reference detection [#12](#12) ([991c90a](991c90a))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants