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

Dissolve state objects that don't deal with state #301

Open
dphfox opened this issue Jan 20, 2024 · 4 comments
Open

Dissolve state objects that don't deal with state #301

dphfox opened this issue Jan 20, 2024 · 4 comments
Labels
enhancement New feature or request not ready - blocked Waiting on other work to be done

Comments

@dphfox
Copy link
Owner

dphfox commented Jan 20, 2024

Right now, we encourage users to create Computed objects whenever they're dealing with possible state objects, e.g.

local function sum(
    a: Fusion.CanBeState<number>,
    b: Fusion.CanBeState<number>
): Fusion.StateObject<number>
    return Computed(function(use)
        return use(a) + use(b)
    end)
end

local foo = sum(2, 2)
print(foo) --> table: 0x12345789abcdef
print(peek(foo)) --> 4

However, in these scenarios, you can often end up with computations that deal entirely with constant values, for which a state object is completely redundant.

In these cases, I propose that state object constructors should be able to return constant values if they can prove that they will evaluate to a constant value. This is definitely a breaking change, but I feel that our ability to interchangeably handle constants and state objects should make it trivial to work with.

The above example becomes:

local function sum(
    a: Fusion.CanBeState<number>,
    b: Fusion.CanBeState<number>
): Fusion.CanBeState<number>
    return Computed(function(use)
        return use(a) + use(b)
    end)
end

local foo = sum(2, 2)
print(foo) --> 4
print(peek(foo)) --> 4

The allocation of the object can be completely avoided, lowering memory usage. State object constructors are re-conceptualised as suggestions that something may change, which Fusion can optimise away.

@dphfox dphfox added enhancement New feature or request not ready - evaluating Currently gauging feedback labels Jan 20, 2024
@dphfox
Copy link
Owner Author

dphfox commented Jan 20, 2024

In combination with #300, this also means that observers of constant values would be reduced to a table of either no-ops or functions that execute whatever callback is given to them.

@dphfox
Copy link
Owner Author

dphfox commented Jan 20, 2024

There's a really clever way to do this under both eager and lazy execution (#144) - at the very start of the Computed constructor, run the processor function in a pcall with a use function that passes through constants and errors when given a state object. If the pcall resolves to a value, then the Computed constructor is proven to always be that value, and so we can early return it without even touching the allocating code.

This still obeys the Fusion.Use type signature, and shouldn't lead to any issues wrt #270 because the moment that the calculation becomes non-constant, it's instantly terminated and so should never be invalidated by an inconsistent reactive graph.

@dphfox dphfox moved this to Prospective in Fusion 0.3 Jan 20, 2024
@dphfox
Copy link
Owner Author

dphfox commented Jan 20, 2024

It's probably important to clarify that this optimisation only applies to state objects that don't specify any additional methods. For example, while Tween could be dissolved away, Spring could not, because Spring has methods for setting position and velocity.

@dphfox dphfox added not ready - blocked Waiting on other work to be done and removed not ready - evaluating Currently gauging feedback labels Feb 3, 2024
@dphfox dphfox pinned this issue Apr 16, 2024
@dphfox
Copy link
Owner Author

dphfox commented Apr 21, 2024

I'm probably going to mark this issue as blocked again for the time being. Considerations around nested state objects nullify a bit of the benefit of a change like this, and Fusion isn't really ready to tackle unknown nesting levels quite yet. Luau's type checking probably wouldn't work well either.

So I'll keep this around as a longterm goal instead.

@dphfox dphfox removed this from Fusion 0.3 Apr 21, 2024
@dphfox dphfox unpinned this issue Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request not ready - blocked Waiting on other work to be done
Projects
None yet
Development

No branches or pull requests

1 participant