Neos-Metaverse / NeosPublic

A public issue/wiki only repository for the NeosVR project
193 stars 9 forks source link

New Node: Sort #3703

Open JackTheFoxOtter opened 2 years ago

JackTheFoxOtter commented 2 years ago

Is your feature request related to a problem? Please describe.

Currently, if you have a fixed number of values you want to sort, a sorting algorithm has to be implemented manually.

Relevant issues

No response

Describe the solution you'd like

A "Sort" node, which has 2-N value inputs and an equal number of outputs. The outputs would contain the same set of input values, just sorted from lowest on index 0 to highest on index N-1. Additionally there should be an additional boolean input to specify wether the sorting direction should be revesed or not.

Describe alternatives you've considered

Alternatives are writing the sorting / comparison manually, which is fine for small amounts of values but can quickly get messy with many of them.

Additional context

This node proposal is for situations in which you have a fixed amount of values you want to sort. Sorting a flexible amount of values would probably best be done with a "Sort List" node once list support is added. However, even when that exists further down the line, I do think this node is still a valid proposal considering you likely don't always want to use lists if you don't strictly speaking have to.

ProbablePrime commented 2 years ago

This is an unfortunate case of "needs collections", which I see is listed in your description but that's just how things go sometimes.

We could implement this node, but it would immediately be superseded in every way by collections. In these cases we have to balance the effort to add the node now, vs the effort to add it after collections.

JackTheFoxOtter commented 2 years ago

Well, as stated in my comment, this proposal is explicitly for use without collections. Where you have a fixed amount of values you want to sort, it would be a node with multiple inputs but not a collection. I think both are valuable but serve different purposes. I might have a LogiX network with 4 independent values which I need to sort for further processing. That amount wouldn't change once the node graph is done, since it's part of an internal computation, hence no need to have a flexible list of values to sort.

Also, depending on how collections will be implemented, you would first have to turn the 4 individual values into a collection, then sort that collection, and then separate it out into it's invididual components again since that's what you need for the next step of the node graph. So there will likely be a minimum of 3 collection-based nodes required to achieve the same thing as the single node in this proposal can achieve.

It would also probably be faster internally, as dynamic lists tend to come with a performance overhead.

ProbablePrime commented 2 years ago

I understand, it just needs collections to be evaluated before we'll consider taking a look here.|

Its still a good suggestion, I'm just setting expectations as best as I can.

Frozenreflex commented 2 years ago
using System;
using System.Linq;
using BaseX;
using FrooxEngine.UIX;

namespace FrooxEngine.LogiX.Operators
{
    [NodeName("Sort")]
    [Category("LogiX/Math")]
    [NodeDefaultType(typeof(Sort<float>))]
    public class Sort<T> : LogixNode where T : IComparable
    {
        public readonly SyncList<Input<T>> ValueInputs;
        public readonly Input<bool> Backward;
        public readonly SyncList<Output<T>> ValueOutputs;
        protected override void OnAttach()
        {
            base.OnAttach();
            for (var i = 0; i < 2; i++)
            {
                ValueOutputs.Add();
                ValueInputs.Add();
            }
        }
        protected override void OnEvaluate()
        {
            var inputs = ValueInputs.Select(t => t.EvaluateRaw()).ToList();
            inputs.Sort();
            if (Backward.EvaluateRaw()) inputs.Reverse();
            for (var i = 0; i < ValueOutputs.Count; i++) ValueOutputs[i].Value = inputs[i];
        }
        protected override void OnGenerateVisual(Slot root)
        {
            var uIBuilder = GenerateUI(root);
            uIBuilder.Panel();
            uIBuilder.HorizontalFooter(32f, out var footer, out var _);
            var uIBuilder2 = new UIBuilder(footer);
            uIBuilder2.HorizontalLayout(4f);
            LocaleString text = "+";
            var tint = color.White;
            uIBuilder2.Button(in text, in tint, Add);
            text = "-";
            tint = color.White;
            uIBuilder2.Button(in text, in tint, Remove);
        }
        [SyncMethod]
        private void Add(IButton button, ButtonEventData eventData)
        {
            ValueInputs.Add();
            ValueOutputs.Add();
            RefreshLogixBox();
        }
        [SyncMethod]
        private void Remove(IButton button, ButtonEventData eventData)
        {
            if (ValueInputs.Count <= 2) return;
            ValueInputs.RemoveAt(ValueInputs.Count - 1);
            ValueOutputs.RemoveAt(ValueOutputs.Count - 1);
            RefreshLogixBox();
        }
    }
}

image

ProbablePrime commented 2 years ago

Thank you, but we're not able to accept 3rd party node contributions at this time.