Gracefully scheduling the same runnable twice

[krispya]

Currently if the same runnable is scheduled twice it throws an error.

add(schedule, aFn, id('A'));
add(schedule, aFn, id('A'));

In imperative apps this probably isn't an issue, you just check to make sure you are scheduling the runnable once, but in dynamic apps like React this can be a real issue. Here are two scenarios where I think it is legitimate to handle this case more gracefully than an error:

1. A user needs to reschedule a runnable. Here it would be ideal to get feedback that it is already scheduled and needs to be removed and readded instead of an error.
2. A schedule call is part of a React component and is called each time a view instance is rendered even though it only needs to mount once.

(2) is relevant since a React component is intended to have all of its behavior and state included with the view. The scheduler, especially with a hook, allows for including global level behavior, usually to do with a simulation.

My proposed solution is to have add return a boolean where true is successfully adding and false is unsuccessful, meaning it was already in the system. Then this can be used as feedback for determining what to do next, either ignore or remove/add to reschedule.

[bbohlender]

Imo scheduled functions are resources with a lifetime (add and remove) and therefore for the useSchedule API I've seen in our meeting, I would believe it automatically unschedules the old function and schedules the new function if new != old. Since I am used to using line onFrame functions using useFrame, which as I understand has no relevance to this issue, I don't see any problem in use cases I am very familiar with.

The use case I saw in our meeting regarding ECS, where the Entity react-component schedules the system (if I remember correctly), would be, as I understand, the problem. Can you post the API you showed me that would cause an issue in this case? In general, scheduling the same function twice seems like an anti-pattern from my gut feeling, but the innovations I saw in your presentation might change my gut feeling :)

[krispya]

useFrame will be powered by this scheduler in the future so there is some relevance!

The case we are talking about right now is a classic spawn pattern. You have a React component that encapsulates behavior and view for that feature. Here it is a body in my n-body simulation. It schedules an atomic runnable that syncs the simulation data with the Three scene created by R3F. (I know the implementation details are sparse here but let's not get bogged down in that.) It is added inside the Body function to ensure that even if just one Body is mounted the correct behavior runs.

function Body({ children}) {
    // Schedule an atomic sync runnable.
    useSchedule(syncBodiesWithThree, { after: 'update', before: 'render' })

    return (
        <mesh>
            <circleGeometry args={[1, 32]} />
            <meshBasicMaterial color="red" />
            {children}
        </mesh>
    )
}

And then is spawned up the tree as a list.

// Just creates a list of 100 Body instances. 
function Bodies() {
    return spawn(Body, 100)
}

Now the question is if this is correct.

Coming from the perspective of "I just want to mount Body and have it work", you would want the useSchedule call to be inside of it. But then you need to ref count so when it is mounted n-times you don't get n instances of the same function running when it is meant to be global. Currently, Direct will just throw an error saying it is bad code to try to schedule the same runnable twice. You made a mistake in how you thought your app should be constructed.

The alternative to to force this runnable to be mounted separate from the Body, in this case it would be in Bodies since that i the nearest scope in the app that mounts the logic once.

function Bodies() {
    // Called only once. If Bodies is mounted twice, it needs to be moved up again.
    useSchedule(syncBodiesWithThree, { after: 'update', before: 'render' })
    return spawn(Body, 100)
}

function Body({ children}) {
    return (
        <mesh>
            <circleGeometry args={[1, 32]} />
            <meshBasicMaterial color="red" />
            {children}
        </mesh>
    )
}

This is technically more correct, but also means that there is a DX challenge where it is no longer enough to mount the React component for it to work. You have an additional function you need to import and schedule at a parent level to get it to work, just like a context provider. This isn't terrible, but I have been resistant.

Think of it in terms of a FollowCamera component. Usually you would expect to just mount it and have its behavior work. Otherwise, you would need to also import a global update function and schedule it at some top level, again like a context.

There might be an alternative solution though that doesn't violate React principles where components like a FollowCamera mount local scope runnable purely unless a global scope one is scheduled and then it disables the local scope one.

[krispya]

Following up on this last idea, here is a pseudocode sketch of what that could look like.

function FollowCamera() {
    const ref = useRef(null)

    // Has some global store, eg Zustand or ECS
    const { target } = useGlobalStore()

    // Get the schedule from the R3F state
    const schedule = useThree().schedule
    // Detect if global system is loaded
    const hasGlobalystem = schedule.has(updateFolowCamera)

    // If the global system is not loaded, run a local system instead
    useFrame(!hasGlobalystem && () => {
        // Run a local system to follow the target.
    })

    return <perspectiveCamera ref={ref} />
}

@akdjr What do you think? As a pattern it would ensure that React components run pure local systems unless a global system is loaded at some root scope in the React tree. I think this would address both of our concerns.

[Ctrlmonster]

I've personally always preferred to have my systems live outside the "Entity React Component". It feels anti-ecs to couple global systems together with the view, that's why I've always done sth like the following and treated anything living inside an entity React components as purely local (i.e. a classical useFrame – with the option to be scheduled in between certain global systems):

function Player() {
   useFrame(() => {
     // do something here 
     // gets mounted as many times as <Player> mount
   }, {after: 'UpdatePositions', before: 'render'})
   return ...
}

function Scene() {
  return (
    <>
      {/*scene and world content*/}
      <Player />
      <Enemies />

      {/*global systems – all in one place*/}
      <UpdatePositions.System />
      <UpdateAI.System/>
      <CalcHitPoints.System />
    </>
  )
}

But I can see how supporting an ecosystem has different demands, where people would want their package to be consumed as easily as possible. For this I think your last proposal works quite well, scheduling a local system given that a certain global one doesn't exist.

Regarding throwing an error on duplicate mounting: I'm wondering if users aren't better served if you encourage them to import global systems separately (decoupled from the View). Not knowing exactly which systems are running (because they are nested within React component) is a huge source of mental overhead when debugging (because you have to jump between multiple files) and can be quite frustrating ime. Even worse if the systems come coupled inside library React components. That means you have to copy-paste the source and extract the system yourself if you want your systems to live in one place (or you just want a specific View but not the system to mount). Updating the source yourself means you're now cut off from library updates.

Actually throwing an error if global systems get loaded up multiple times could help in this regard as it forces you to mount the global system on a higher level of the tree (just like Context) and keep the inner content of your entity React components limited to local systems. Neither from the pov of users nor library authors does this seem terribly complex. For the library author it just means you need to export your global systems separately and for the user it means having to add more than one React component to your app. Given the complexity reduction and that it's still just a copy-paste action to get started with a specific library/abstraction that seems worth it to me.

[bbohlender]

Imo the Body and FollowCamera examples both would need a reference to the local threejs so scheduling a global function to update the threejs objects wouldn't be possible. I know this sounds like nitpicking, but I believe it shows that scheduling global/shared functions in multiple components is very rare.

One case I can imagine with the FollowCamera, is if the ref to the FollowCamera was written to a global store. Now scheduling a global function that uses the global store to update the camera would work and using the FollowCamera component twice would be a problem. However, storing reference into a global store seems like a anti pattern and scheduling a local function that updates the local ref is much more robust because its more aligned with react.

As a result I still have no strong opinion, I that the the developer could have the freedom to check if something is scheduled, but I'd also make sure not to advertise these patterns.

[krispya]

Okay, general consensus so far is don't hide global subscriptions inside of React components. Let this be something the user explicitly does themselves so they have full visibility of global behavior. In a sense, the scheduler loop is a model and React is the view and we are stating that there are computations that are not appropriately described in the view itself.

Imo the Body and FollowCamera examples both would need a reference to the local threejs so scheduling a global function to update the threejs objects wouldn't be possible. I know this sounds like nitpicking, but I believe it shows that scheduling global/shared functions in multiple components is very rare.

One case I can imagine with the FollowCamera, is if the ref to the FollowCamera was written to a global store. Now scheduling a global function that uses the global store to update the camera would work and using the FollowCamera component twice would be a problem. However, storing reference into a global store seems like a anti pattern and scheduling a local function that updates the local ref is much more robust because its more aligned with react.

As a result I still have no strong opinion, I that the the developer could have the freedom to check if something is scheduled, but I'd also make sure not to advertise these patterns.

This isn't an anti-pattern -- we have all sorts of uses for needing a ref to the view. A follow camera is a classic example since you need the target ref to even begin the algo. Traditionally you would create a context and pass the ref up and then read from it for the FollowCamera, or you would use Zustand to store it and read it directly. ECS cuts out of the middle-man by making the entire view queryable. In a typical document-like scenario you can usually colocate whatever needs references to each other in the same branch of the tree, but in a dynamic sim like a 3D world there is likely no parent-child relation.

In our React game examples this kind of pattern is very common.

Another benefit is that any behavior extracted as an atomic function can now be reused anywhere, not just inside of React. So I expect users making games or any app that needs to work with an authoritative server or workers to work in this direction. Having a pattern for how to deal with this incremental enhancement will be important, not even just for ECS. This is a problem right now with Zustand, Jotai, etc., where these are global stores and people bake some behavior into the compute functions, but if they need to run a loop instead of something event based they are at a loss. Here we offer a pattern for how to work with global stores per tick in a React-compliant way.

[krispya]

@akdjr I ran into an annoying side effect of the erroring: HMR. Each time a file that schedules tries to HMR it errors.