Coder Social home page Coder Social logo

Comments (14)

dphfox avatar dphfox commented on May 20, 2024 2

I think it could be interesting to wrap the keys in a table to hold extra metadata, for example execution order. Perhaps it would end up looking something like this if you were to make your own OnEvent implementation:

local function OnEvent(eventName)
    return {
        order = "end",
        callback = function(value, props, instance, tasks)
            table.insert(tasks, instance[eventName]:Connect(value))
        end
    }
end

Or perhaps even something that applies some font settings:

local ApplyFont = {
    order = "start",
    callback = function(value, props, instance, tasks)
        props.TextScaled = true
        props.Font = "Gotham"
        props.TextSize = 16
    end
}

In general I'm on board with the idea of separating out the functionality of OnEvent, OnChange, Children, Ref etc from the New function and making it more modular (especially if this allows for third party additions!) but I'm unsure exactly what these functions should be given access to or how exactly they should work.

from fusion.

boatbomber avatar boatbomber commented on May 20, 2024 1

I had an idea on how to do this with extreme simplicity, although it won't cover all cases. It works for my uses though (and the example Tag from OP), so I'll be slapping this into my fork until the official solution is implemented. Figured it's worth sharing.

	--[[
		STEP 2.3: Function keys
	]]
	elseif typeof(key) == "function" then
		-- Defer so instance setup completes before the function runs
		task.defer(key, ref.instance, value)

Edit: A few things of note.

  • This solution works well for constants, but not state.
    This defer trick, although a nice one line solution, doesn't play nicely with dependency management afaik.
    Also, we may want to pass the state's value to the user funciton, rather than the state itself, for the sake of making the user function's easer to create and maintain. Something like this:
    task.defer(key, ref.instance, (typeof(value) == "table" and value.type == "State") and value:get() or value)
  • This makes it run after events are connected, which may not be desirable in many cases.

from fusion.

astrealRBLX avatar astrealRBLX commented on May 20, 2024 1

I don't believe exposing the API this much is the correct implementation of this feature. Fusion becoming modular is important but I believe that this can quickly get confusing. I much prefer the idea of having a dedicated API token for use in the New function that will expose Fusion's API. For example:

local function ApplyFont(value, props, instance, tasks) -- Value() objects are not automatically unwrapped
    ...
end

local function OnEvent(eventName)
    return function(value, props, instance, tasks)
        ...
    end
end

New 'TextLabel' {
    ...
    
    [API] = {
        [ApplyFont] = Enum.Font.Gotham,
        [OnEvent 'Activated'] = ...
    }
}

The API token expects its value to be an array where the indices define callback order. For safer and simpler operations I believe a Preprocess and Postprocess token should exist similar to the API token. The difference being what they actually have access to. I don't really like splitting this functionality up into two different tokens but I can't think of a nicer way of going about it.

local function Tag(inst, props)
    if type(props) == "table" then
        for _, tag in ipairs(props) do
            CollectionService:AddTag(inst, tag)
        end
    else
        CollectionService:AddTag(inst, props)
    end
end

New 'TextLabel' {
    ...
    
    [Preprocess] = { -- Applied after step 1 but before step 2
        [Tag] = {'Hello', 'World'} -- If a Value() is used it should automatically be unwrapped when passed to the preprocessor
    },
    
    [Postprocess] = { ... } -- Works the same except applied after step 6
}

from fusion.

astrealRBLX avatar astrealRBLX commented on May 20, 2024 1

Currently Fusion uses symbols for "special" keys and I believe exposing the API to certain functions should be no different. When wrapped in the API call the passed function will be called with whatever API parameters are exposed. If a raw function is used as a key then it should behave like what the initial issue proposed where it receives only the instance and props. I should've been more clear as to what I was trying to get at hopefully that makes more sense.

from fusion.

dphfox avatar dphfox commented on May 20, 2024

This is an interesting idea! I've been looking for ways to make the New function more modular and extensible - in particular, by end users - and this has potential to be the ticket to simple, clean, extensible and understandable code.

I suspect if we did this, it would almost completely replace symbol keys - there's still some questions about how we'd deal with order of operations though (e.g. events have to be connected last)

Ill have to investigate this further 👍🏻

from fusion.

Gargafield avatar Gargafield commented on May 20, 2024

This is an interesting idea! I've been looking for ways to make the New function more modular and extensible - in particular, by end users - and this has potential to be the ticket to simple, clean, extensible and understandable code.

I suspect if we did this, it would almost completely replace symbol keys - there's still some questions about how we'd deal with order of operations though (e.g. events have to be connected last)

Ill have to investigate this further 👍🏻

The function needs to run last since they need the instance. There's also the possibility that classes could have a "RunPriority" key.

from fusion.

dphfox avatar dphfox commented on May 20, 2024

I don't believe exposing the API this much is the correct implementation of this feature. Fusion becoming modular is important but I believe that this can quickly get confusing.

What specifically would you find confusing about the proposed implementation? If we can establish what's wrong with it more definitively, we can work to solve those issues in particular :)

New 'TextLabel' {
    ...
    
    [API] = {
        [ApplyFont] = Enum.Font.Gotham,
        [OnEvent 'Activated'] = ...
    }
}

I really don't like this idea - if we were to go ahead with this, then every single use of a non-string key would have to be wrapped in [API]. This is a massive breaking change. If the purpose is to distinguish what is a regular property assignment and what is special behaviour, the use of a non-string key already indicates this perfectly well. It's nice that users can specify their own order of operations here, but in the vast majority of cases users don't care and want the library to handle this for them, for example with events.

from fusion.

astrealRBLX avatar astrealRBLX commented on May 20, 2024

Alright your argument makes sense + I think code could get cluttered very quickly. When I said things can get confusing what I specifically meant is I don't believe exposing such a large portion of the Fusion API for something as simple as applying tags makes sense. It's worth looking into a proper way to provide a more limited version of the API for simpler and safer tasks like that.

In terms of the actual syntax I had proposed before I think this might work better.

New 'TextLabel' {
    [API(Tag)] = {'Hello', 'World'},
    [API(ApplyFont)] = Enum.Font.Gotham,
    [API(OnEvent 'Activated')] = ...
}

Then question then becomes what should be returned by the function fed into the API() function.

from fusion.

dphfox avatar dphfox commented on May 20, 2024

When I said things can get confusing what I specifically meant is I don't believe exposing such a large portion of the Fusion API for something as simple as applying tags makes sense.

This is a fair point - I'm mainly thinking about third party libraries with these changes, but the tags are simply something easy that people could add in. I'm currently looking to make Fusion much easier to extend to fit a wider range of coding styles and provide custom features that feel as good as first-party features.

New 'TextLabel' {
    [API(Tag)] = {'Hello', 'World'},
    [API(ApplyFont)] = Enum.Font.Gotham,
    [API(OnEvent 'Activated')] = ...
}

What is the purpose of passing everything through an API call here?

from fusion.

dphfox avatar dphfox commented on May 20, 2024

I still don't really understand what you're trying to do here :/

from fusion.

Gargafield avatar Gargafield commented on May 20, 2024

I like @astrealRBLX approach. But it's still a bit cluttered.
Here's my idea:
You can set functions as keys and they will be called immediately after creating the instance. The value of the key and the instance is passed to the function.
Using the API function you can add special metadata. Like when the function should be called. Or if it wants any other data, like setting internal values in New.

from fusion.

Gargafield avatar Gargafield commented on May 20, 2024

There's a lot of feature request that could be fixed by this issue. It would be better to get 5 components from GitHub than 5 extra functions in fusion.

from fusion.

dphfox avatar dphfox commented on May 20, 2024

Right now I'm going forward with this design for the New rewrite:

local Children = {
    type = "SpecialKey", -- identifies this as a special/function key
    kind = "Children", -- for type checking/debugging
    step = "descendants", -- when should this key be run during the property application process?
    apply = function() ... end -- applies this to the instance
}

I'm willing to revisit this in the future, but right now this is the design I have the most confidence in. Plus, we have to settle on something for now.

from fusion.

dphfox avatar dphfox commented on May 20, 2024

I think we've settled on the current design for now.

from fusion.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.