Skip to content

Commit

Permalink
Reduce the number of checks and special code paths in Fusion (#418)
Browse files Browse the repository at this point in the history
  • Loading branch information
dphfox authored Feb 1, 2025
1 parent 6da9463 commit eb6112c
Show file tree
Hide file tree
Showing 24 changed files with 13 additions and 502 deletions.
175 changes: 0 additions & 175 deletions docs/api-reference/general/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,24 +151,6 @@ be designed to have dependents or dependencies.

<div class="fusiondoc-error-api-section" markdown>

## cleanupWasRenamed

```
`Fusion.cleanup` was renamed to `Fusion.doCleanup`. This will be an error in
future versions of Fusion.
```

**Thrown by:**
[`doCleanup`](../../memory/members/docleanup)

You attempted to use `cleanup()` in Fusion 0.3, which replaces it with the
`doCleanup()` method.
</div>

-----

<div class="fusiondoc-error-api-section" markdown>

## destroyedTwice

```
Expand Down Expand Up @@ -197,34 +179,6 @@ cleaning up the scope.

<div class="fusiondoc-error-api-section" markdown>

## destructorRedundant

```
Computed destructors no longer do anything. If you wish to run code on destroy,
`table.insert` a function into the `scope` argument. See discussion #292 on
GitHub for advice.
```

**Thrown by:**
[`Computed`](../../state/members/computed),
[`ForKeys`](../../state/members/forkeys),
[`ForValues`](../../state/members/forvalues),
[`ForPairs`](../../state/members/forpairs)

**Related discussions:**
[`#292`](https://github.com/dphfox/Fusion/discussions/292)

You passed an extra parameter to the constructor, which has historically been
interpreted as a function that runs when a value is cleaned up.

This mechanism has been replaced by
[scopes](../../../tutorials/fundamentals/scopes).
</div>

-----

<div class="fusiondoc-error-api-section" markdown>

## forKeyCollision

```
Expand Down Expand Up @@ -371,23 +325,6 @@ type.

<div class="fusiondoc-error-api-section" markdown>

## invalidRefType

```
Instance refs must be Value objects.
```

**Thrown by:**
[`Ref`](../../roblox/members/ref)

`Ref` expected you to give it a [value](../../state/members/value), but you gave
it something else.
</div>

-----

<div class="fusiondoc-error-api-section" markdown>

## invalidSpringDamping

```
Expand Down Expand Up @@ -533,54 +470,6 @@ reference to it, as it is no longer valid.

<div class="fusiondoc-error-api-section" markdown>

## possiblyOutlives

```
The Computed (bound to the PaddingLeft property) will be destroyed before the
UIPadding instance; the latter is in a different scope that gets destroyed too
quickly. To fix this, review the order they're created in, and what scopes they
belong to. See discussion #292 on GitHub for advice.
```

**Thrown by:**
[`Spring`](../../animation/members/spring),
[`Tween`](../../animation/members/tween),
[`New`](../../roblox/members/new),
[`Hydrate`](../../roblox/members/hydrate),
[`Attribute`](../../roblox/members/attribute),
[`AttributeOut`](../../roblox/members/attributeout),
[`Out`](../../roblox/members/out),
[`Ref`](../../roblox/members/ref),
[`Computed`](../../state/members/computed),
[`Observer`](../../graph/members/observer)

**Related discussions:**
[`#292`](https://github.com/dphfox/Fusion/discussions/292)

If you use an object after it's been destroyed, then your code can break. This
mainly happens when one object 'outlives' another object that it's using.

Because [scopes](../../../tutorials/fundamentals/scopes) clean up the newest
objects first, this can happen when an old object depends on something much
newer that itself. During cleanup, a situation could arise where the newer
object is destroyed, then the older object runs code of some kind that needed
the newer object to be there.

Fusion can check for situations like this by analysing the scopes. This message
is shown when Fusion can prove one of these situations will occur.

There are two typical solutions:

- If the objects should always be created and destroyed at the exact same time,
then ensure they're created in the correct order.
- Otherwise, move the objects into separate scopes, and ensure that both scopes
can exist without the other scope.
</div>

-----

<div class="fusiondoc-error-api-section" markdown>

## propertySetError

```
Expand All @@ -602,39 +491,6 @@ issue.

<div class="fusiondoc-error-api-section" markdown>

## scopeMissing

```
To create Observers, provide a scope. (e.g. `myScope:Observer(watching)`). See
discussion #292 on GitHub for advice.
```

**Thrown by:**
[`New`](../../roblox/members/new),
[`Hydrate`](../../roblox/members/hydrate),
[`Value`](../../state/members/value),
[`Computed`](../../state/members/computed),
[`Observer`](../../graph/members/observer),
[`ForKeys`](../../state/members/forkeys),
[`ForValues`](../../state/members/forvalues),
[`ForPairs`](../../state/members/forpairs),
[`Spring`](../../animation/members/spring),
[`Tween`](../../animation/members/tween)

**Related discussions:**
[`#292`](https://github.com/dphfox/Fusion/discussions/292)

You attempted to create an object without providing a
[scope](../../../tutorials/fundamentals/scopes) as the first parameter.

Scopes are mandatory for all Fusion constructors so that Fusion knows when the
object should be destroyed.
</div>

-----

<div class="fusiondoc-error-api-section" markdown>

## springNanGoal

```
Expand Down Expand Up @@ -696,36 +552,6 @@ is currently outputting. However, you provided a different data type.

<div class="fusiondoc-error-api-section" markdown>

## stateGetWasRemoved

```
`StateObject:get()` has been replaced by `use()` and `peek()` - see discussion
#217 on GitHub.
```

**Thrown by:**
[`Value`](../../state/members/value),
[`Computed`](../../state/members/computed),
[`ForKeys`](../../state/members/forkeys),
[`ForValues`](../../state/members/forvalues),
[`ForPairs`](../../state/members/forpairs),
[`Spring`](../../animation/members/spring),
[`Tween`](../../animation/members/tween)

**Related discussions:**
[`#217`](https://github.com/dphfox/Fusion/discussions/217)

Older versions of Fusion let you call `:get()` directly on state objects to read
their current value and attempt to infer dependencies.

This has been replaced by [use functions](../../state/types/use) in Fusion 0.3
for more predictable behaviour and better support for constant values.
</div>

-----

<div class="fusiondoc-error-api-section" markdown>

## tweenNanGoal

```
Expand Down Expand Up @@ -860,7 +686,6 @@ is use()-ing. See discussion #292 on GitHub for advice.
[`Attribute`](../../roblox/members/attribute),
[`AttributeOut`](../../roblox/members/attributeout),
[`Out`](../../roblox/members/out),
[`Ref`](../../roblox/members/ref),
[`Computed`](../../state/members/computed),
[`Observer`](../../graph/members/observer)

Expand Down
9 changes: 0 additions & 9 deletions src/Animation/Spring.luau
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,6 @@ local function Spring<T>(
damping: Types.UsedAs<number>?
): Types.Spring<T>
local createdAt = os.clock()
if typeof(scope) ~= "table" or castToState(scope) ~= nil then
External.logError("scopeMissing", nil, "Springs", "myScope:Spring(goalState, speed, damping)")
end

local goalState = castToState(goal)
local stopwatch = nil
Expand Down Expand Up @@ -165,12 +162,6 @@ function class.addVelocity<T>(
change(self)
end

function class.get<T>(
self: Self<T>
): never
return External.logError("stateGetWasRemoved")
end

function class.setPosition<T>(
self: Self<T>,
newValue: T
Expand Down
9 changes: 0 additions & 9 deletions src/Animation/Tween.luau
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ local function Tween<T>(
tweenInfo: Types.UsedAs<TweenInfo>?
): Types.Tween<T>
local createdAt = os.clock()
if castToState(scope) then
External.logError("scopeMissing", nil, "Tweens", "myScope:Tween(goalState, tweenInfo)")
end

local goalState = castToState(goal)
local stopwatch = nil
Expand Down Expand Up @@ -118,12 +115,6 @@ local function Tween<T>(
return self
end

function class.get<T>(
self: Self<T>
): never
return External.logError("stateGetWasRemoved")
end

function class._evaluate<T>(
self: Self<T>
): boolean
Expand Down
12 changes: 0 additions & 12 deletions src/External.luau
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ local ERROR_INFO_URL = "https://elttob.uk/Fusion/0.3/api-reference/general/error

local External = {}

-- Indicates that a highly time-critical passage of code is running. During
-- critical periods of a program, Fusion might decide to change some of its
-- internal behaviour to be more performance friendly.
local timeCritical = false

-- Multiplier for running-time safety checks across the Fusion codebase. Used to
-- stricten tests on infinite loop detection during unit testing.
External.safetyTimerMultiplier = 1
Expand All @@ -47,13 +42,6 @@ function External.setExternalProvider(
return oldProvider
end

--[[
Returns true if a highly time-critical passage of code is running.
]]
function External.isTimeCritical(): boolean
return timeCritical
end

--[[
Sends an immediate task to the external provider. Throws if none is set.
]]
Expand Down
3 changes: 0 additions & 3 deletions src/Graph/Observer.luau
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ local function Observer(
watching: unknown
): Types.Observer
local createdAt = os.clock()
if watching == nil then
External.logError("scopeMissing", nil, "Observers", "myScope:Observer(watching)")
end

local self: Self = setmetatable(
{
Expand Down
4 changes: 0 additions & 4 deletions src/Instances/Hydrate.luau
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,12 @@ local task = nil -- Disable usage of Roblox's task scheduler

local Package = script.Parent.Parent
local Types = require(Package.Types)
local External = require(Package.External)
local applyInstanceProps = require(Package.Instances.applyInstanceProps)

local function Hydrate(
scope: Types.Scope<unknown>,
target: Instance
)
if target :: any == nil then
External.logError("scopeMissing", nil, "instances using Hydrate", "myScope:Hydrate (instance) { ... }")
end
return function(
props: Types.PropertyTable
): Instance
Expand Down
5 changes: 0 additions & 5 deletions src/Instances/New.luau
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ local function New(
scope: Types.Scope<unknown>,
className: string
)
if (className :: any) == nil then
local scope = (scope :: any) :: string
External.logError("scopeMissing", nil, "instances using New", "myScope:New \"" .. scope .. "\" { ... }")
end

-- This might look appealing to try and cache. But please don't. The scope
-- upvalue is shared between the two curried function calls, so this will
-- open incredible cross-codebase wormholes like you've never seen before.
Expand Down
6 changes: 0 additions & 6 deletions src/Logging/messages.luau
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ return {
cannotConnectEvent = "The %s class doesn't have an event called '%s'.",
cannotCreateClass = "Can't create a new instance of class '%s'.",
cannotDepend = "%s can't depend on %s.",
cleanupWasRenamed = "`Fusion.cleanup` was renamed to `Fusion.doCleanup`. This will be an error in future versions of Fusion.",
destroyedTwice = "`doCleanup()` was given something that it is already cleaning up. Unclear how to proceed.",
destructorRedundant = "%s destructors no longer do anything. If you wish to run code on destroy, `table.insert` a function into the `scope` argument. See discussion #292 on GitHub for advice.",
forKeyCollision = "The key '%s' was returned multiple times simultaneously, which is not allowed in `For` objects.",
infiniteLoop = "Detected an infinite loop. Consider adding an explicit breakpoint to your code to prevent a cyclic dependency.",
invalidAttributeChangeHandler = "The change handler for the '%s' attribute must be a function.",
Expand All @@ -26,7 +24,6 @@ return {
invalidOutProperty = "The %s class doesn't have a property called '%s'.",
invalidOutType = "[Out] properties must be given Value objects.",
invalidPropertyType = "'%s.%s' expected a '%s' type, but got a '%s' type.",
invalidRefType = "Instance refs must be Value objects.",
invalidSpringDamping = "The damping ratio for a spring must be >= 0. (damping was %.2f)",
invalidSpringSpeed = "The speed of a spring must be >= 0. (speed was %.2f)",
mergeConflict = "Multiple definitions for '%s' found while merging.",
Expand All @@ -35,13 +32,10 @@ return {
mistypedTweenInfo = "The tween info of a tween must be a TweenInfo. (got a %s)",
noTaskScheduler = "Fusion is not connected to an external task scheduler.",
poisonedScope = "Attempted to use a scope after it's been destroyed; %s",
possiblyOutlives = "%s will be destroyed before %s; %s. To fix this, review the order they're created in, and what scopes they belong to. See discussion #292 on GitHub for advice.",
propertySetError = "Error setting property:\nERROR_MESSAGE",
scopeMissing = "To create %s, provide a scope. (e.g. `%s`). See discussion #292 on GitHub for advice.",
springNanGoal = "A spring was given a NaN goal, so some simulation has been skipped. Ensure no springs have NaN goals.",
springNanMotion = "A spring encountered NaN during motion, so has snapped to the goal position. Ensure no springs have NaN positions or velocities.",
springTypeMismatch = "The type '%s' doesn't match the spring's type '%s'.",
stateGetWasRemoved = "`StateObject:get()` has been replaced by `use()` and `peek()` - see discussion #217 on GitHub.",
tweenNanGoal = "A tween was given a NaN goal, so some animation has been skipped. Ensure no tweens have NaN goals.",
tweenNanMotion = "A tween encountered NaN during motion, so has snapped to the goal. Ensure no tweens have NaN in their tween infos.",
unknownMessage = "Unknown error:\nERROR_MESSAGE",
Expand Down
12 changes: 1 addition & 11 deletions src/Memory/checkLifetime.luau
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ local task = nil -- Disable usage of Roblox's task scheduler
local Package = script.Parent.Parent
local Types = require(Package.Types)
local External = require(Package.External)
local whichLivesLonger = require(Package.Memory.whichLivesLonger)
local nameOf = require(Package.Utility.nameOf)

local checkLifetime = {}
Expand Down Expand Up @@ -119,16 +118,7 @@ function checkLifetime.bOutlivesA<A, B, Args...>(
)
if scopeB == nil then
External.logError("useAfterDestroy", nil, formatter(a, b, ...))
elseif whichLivesLonger(scopeA, a, scopeB, b) == "definitely-a" then
local aName, bName = formatter(a, b, ...)
External.logWarn(
"possiblyOutlives",
aName, bName,
if scopeA == scopeB then
"they're in the same scope, but the latter is destroyed too quickly"
else
"the latter is in a different scope that gets destroyed too quickly"
)
end
end

return checkLifetime
Loading

0 comments on commit eb6112c

Please sign in to comment.