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

[Refactor] Assume the GDJS renderer may not be present #5667

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

arthuro555
Copy link
Contributor

This PR aims at uncoupling all core runtime classes of GDJS from their renderers. A few things have been done to that effect:

  • The classes of the renderers have been marked as potentially undefined, enforcing the check of their presence before initializing an instance.
  • The type of the classes instances have been marked as potentially undefined, forcing all classes that keeps a reference to a renderer class instance to check for undefined before every usage of a renderer class instance.
  • Renderer classes assume the existence of each other, since they require each other to function, thus they use a non-null type cast (x!), other classes which only send messages to the renderer classes but do not (should not) rely on it check for existence (x?/if(x) x)
  • When the RuntimeScene renderer is not present, the rendering related parts of the event loop (updatePreRender, updatePostRender, render...) are disabled
  • Some state that was previously stored on renderers, like the tint for certain objects, have been moved to the object to be get and set even when a renderer is not present, to not break game logic that might depend on these
  • Dimensions of the resources is crucial in GDJS to determine the base size/scale factor of an object, which in turn determines the AABB of the object, etc. Those dimensions were obtained from the renderer, and this PR addresses this only in-part. The obtaining of the base dimension has been moved to a method in RuntimeGame, and objects adapted to use that instead of the renderer provided base dimensions. However, the method in RuntimeGame still relies on the renderer classes to obtain the base size of a resource. The idea behind that move is that, in a follow-up PR, we will read the resources base size at export time and embed them in the resources project data, and RuntimeGame will read from that instead of relying on the renderers. We cannot put this on the managers of the individual resources types, since those are provided by the renderer.
  • When the RuntimeGame renderer is not present, it will fallback to a setImmediate/setTimeout based game loop. Since there is no rendering when that happens, setImmediate/setTimeout is a better match than requestAnimationFrame since syncing with browser rendering is useless, and it will provide more ticks per second, and it is supported by NodeJS.

Although this PR does not fully adds support for those to GDJS yet, it paves the way for making server builds and multi-threading in GDevelop.

About performance, there may be an impact, but I believe it would be benign, since checks for undefined is something the JS engine is doing anyways on any property access?

Overview of changes that require closer inspection

Those changes were made over multiple weeks so I hope I am not missing any. Here is a list of the parts that were not as simple as "add if (this._renderer) in front of it", and how I decided to handle them:

3D

I have very little experience with working with ThreeJS and the GDevelop 3D extension, and thus 3D is the one thing I haven't really adapted to work without a renderer. I plastered all the places that need to be changed with TODO comments, help would be welcome here! This PR can still be merged in spite of that - this will just need to be taken care of before GDJS truly supports being ran without renderers.

Shape Painter

The shape painter is by nature something graphical. However, its dimensions are defined by what is drawn inside of it, and those can be used for collision checks and may be depended on for the game logic to work properly. However, I believe this is too much of an edge case for it to justify the complexity required to implement computing of bounding box changes independently of the renderer, and thus implemented it as returning 0 for dimensions. I think it is fine to pass of to users as a "Gotcha" of server builds that the shape painter will not draw anything and as such its bounds will remain at 0.

Texts

Texts get their dimensions measured by looking at the texture size of the rendered image of the text. This is not possible without rendering, obviously. On a render-less build, exact text dimensions cannot be obtained, but it could be used for game logic, although ill-advised. To minimize potentially game-breaking impacts of the dimensions not being known, they are approximated by multiplying the amount of lines/the length of the biggest line with the font size when the renderer is not present.

Bitmap text

On other texts, the font size is set as a property of the object, but on Bitmap text, it is defined by the font file. Like the other resources, in a follow up PR we'll embed that font size on the resource data and get it from RuntimeGame. However, it does not yet call RuntimeGame#getResourceBaseDimensions, since when using the renderer to find the dimensions, two resources are needed (the image and bitmap font), however getResourceBaseDimensions cannot know which image to use for a given bitmap font (and it makes no sense for it to take in a second resource).

The bitmap fontsize will need to be adapted in the follow up PR that add export-time measuring of resources to get it from RuntimeGame#getResourceBaseDimensions.

@arthuro555
Copy link
Contributor Author

arthuro555 commented Sep 21, 2023

I would appreciate if this could be reviewed quickly, since it touches a lot of files it'll often create conflicts and takes a lot of effort to maintain mergeable 🙏

@Oxey405
Copy link
Contributor

Oxey405 commented Sep 23, 2023

Looks good to me

@4ian
Copy link
Owner

4ian commented Sep 27, 2023

Thanks for opening this and working toward a "headless" version of the game engine :)

My major concern is that this adds yet another small but existant indirection when calling the renderers. It also makes it harder to see if a renderer was not set while it should have been (you loose the static typing peace of mind that the renderer does exist).
Instead, could it be possible to have "NullRenderer"s or "HeadlessRenderer"s? Do you see any major drawbacks?

This would also allow this HeadlessRenderer to be more explicit about how resource sizes are loaded (either from something stored in the project, or in some other data, or by using mock size, or by reading the files from the disk if running on Node.js...) - what do you think?

@arthuro555
Copy link
Contributor Author

if a renderer was not set while it should have been (you loose the static typing peace of mind that the renderer does exist)

That is one way to look at it... On the other hand, this safety is exchanged with the safety to know that all code will always be able to operate without a renderer

Generally, I had originally thought about making a "Headless Renderer" first before pivoting to this approach. I beleieve it it a better approach because it enforces a hard segregation of the "core" of the engine and the "renderer" of the engine, and plays into the plugable renderer architecture of GDJS.

When possible, code should always be located in the "core", since the behavior of a renderer might differ from that of another (leading to avoidable inconsistencies or even bugs with the introduction of a new renderer), and since the renderer will have multiple implementations, leading to code duplication or having to code the same thing differently multiple times (back to the GDCpp & GDJS problem).

By marking the renderer always as optional, we force all developers to always acknowledge the gap between renderer and "core" whenever they communicate, thus making the programmer less likely to offload a big chunk of work to the renderer, which I consider an anti-pattern as it makes it very difficult to emulate that work when the game works Headlessly or to add a new renderer which might not have that specific option, opting for a function located in-core by default whenever it isn't necessary to couple core functions to the renderer.

There are some moments where it would also be really awkward to have a Headless renderer. For example, some code paths can and should be simply skipped over whenever there is no rendering, for example, the pre-render updates. We'd probably end up marking the Headless renderer with a special isNotActuallyRendering property to check in the game loop, but at this point that's becoming kind of a code smell - if you need to check a variable on an object to know if it is not a "Null" object, maybe the object itself should just be... null?


About the small indirection, I am assuming that's primarily a performance concern. What's especially great about this approach is that we are explicitly marking all the code paths where we communicate with the renderer in a pretty much standardized way:

  • Either accessing the renderer with this._renderer or anything.getRenderer()
  • Checking the existence with either if(this._renderer) or this._renderer?.something

...which means we can find every such code path relatively easily at build time, and transform the code to either:

  • Eliminate the check if we are making a headful build
  • Eliminate the code path if we are making a headless build

In terms of indirection and performance, this would be equivalent to using a Headless renderer on the Headful build, since there is also no additional overhead, but would allow us to drop the overhead of calling noop functions on the renderer on Headless builds of the runtime!
Performance wise, this would be the best decision long term.

We could do this with something like esbuild's define, which many project use to set for example process.env.NODE_ENV at build-time to mark and eliminate development code (e.g. console.logs) from their production bundle among other things.

@4ian
Copy link
Owner

4ian commented Oct 9, 2023

I have yet to answer precisely but just pointing out that PlayCanvas seems to have a NullRenderer since recently, might be interested to look at their approach: https://twitter.com/playcanvas/status/1711392399514038276?t=vy7qDUri7P-HMrvOP6TTHA&s=19

(Not sure if this applies to us or not, just for inspiration)

@arthuro555
Copy link
Contributor Author

BabylonJS also has a similiar approach: https://doc.babylonjs.com/typedoc/classes/BABYLON.NullEngine

@arthuro555
Copy link
Contributor Author

I have yet to answer precisely

I don't mind redoing the PR the way you want if you disagree with my points, I'd appreciate if I just could get just any answer to know how I can proceed 🙏
I don't want to be too pushy since I know you're busy but I've been hanging there waiting for two months now, and people are asking me about THNK multiple times a week, which I cannot really get work done on until I can get an instance of the game to run without rendering 🥺

@MyNameIsRinax
Copy link
Contributor

🙏

@4ian
Copy link
Owner

4ian commented Nov 30, 2023

Let me check this again and make an answer tomorrow on how we proceed :) (probably in the direction of a Null/HeadlessRenderer, as this could allow this renderer to still load images and give image size, or maybe other stuff too).
Thanks for the bump!

@4ian
Copy link
Owner

4ian commented Dec 1, 2023

So my overall thinking about what we should do is:

Use case:

  • What is the primary goal
    => Is this to run on a server to do multiplayer with server authoritative gameplay? What else? You mentioned multithreading?
    => Because this might be long term goals, it's risky to do big changes without being sure how this will be applied. We are on a high risk of being architecture astronauts if we don't have a specific use case in mind.

I do think this is an interesting thing to do, but also I want to be sure that this has an almost immediate application for users so we don't end up maintaining a HeadlessRenderer for 1 year and lose 10% velocity on every task of the game engine for no usage at all.

Technical stuff:

  • Renderer
    => I strongly think we should go for a HeadlessRenderer ("headless" gives an idea of "this is running without a visual rendering - but it can do things like still reading files or computations". "NullRenderer" would rather gives an idea of "this is implementing the interface but doing NOTHING at all, and returning mocks if something is required").
    => I think this is a more common solution than adding .? and ! in the game engine, and allows to extend things (i.e: what I just said about "headless" vs "null").
  • Pragmatic question though before we continue:
    • Are there examples of PixiJS/Three.js headless rendering? In other words, would it be a solution to run the game engine with the existing renderers as-is, without much changes, on a server/Node.js process - but it would still work (maybe render to something in memory, or not render anything at all)
  • State of object instance ("Some state that was previously stored on renderers, like the tint for certain objects, have been moved to the object to be get and set even when a renderer is not present, to not break game logic that might depend on these"
    => Great.
  • Frame lifecycle: "When the RuntimeScene renderer is not present, the rendering related parts of the event loop (updatePreRender, updatePostRender, render...) are disabled"
    => Consider if we should still run them. I think they should absolutely be done, but are just "noop"s. My thinking is that a renderer should introduce as little changes in the control flow and in the game engine in general.
  • Dimensions of the resources (and other I/O)
    => As discussed, I think the renderer could either 1) try to access files or 2) use pre-written sizes, that could be done in the future in exports. If you want to avoid this complexity, you can just use fake size at the beginning.
  • Game loop: "When the RuntimeGame renderer is not present, it will fallback to a setImmediate/setTimeout based game loop"
    => No concerns from me on this side. Though I wonder if running a frame should be something that can be done by the environment running the game with the headless rendering. In other words, maybe we should have something like:
    const runtimeGame = new gdjs.RuntimeGame(gdjs.projectData, {}/* Assume this uses a "HeadlessRenderer" */);
    await game.loadAllAssets(); // Currently this is callback based, but just imagine it's an await for simplicity.
    while (/*some server or game condition*/) { runtemeGame.step(); }
    

Other considerations:

  • "Gotcha" for text/bitmap texts/shape painter
    => I think it's fine to have some objects like these not having the exact same behavior, but in this case we should probably think about a warning in the editor, something like "This action (reading the height of the text) is not 'server safe'".
  • Replayability/determinism: In a perfect world, if a game saves all inputs (Mouse/touch/keyboard/random generation using a seed), this record of input could be used to re-play the exact same gameplay and end result on a "HeadlessRenderer".

@Oxey405
Copy link
Contributor

Oxey405 commented Feb 15, 2024

Any updates on this PR ? Because I would have some use for it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants