-
Notifications
You must be signed in to change notification settings - Fork 716
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
base: master
Are you sure you want to change the base?
Conversation
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 🙏 |
Looks good to me |
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). This would also allow this |
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 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:
...which means we can find every such code path relatively easily at build time, and transform the code to either:
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! We could do this with something like esbuild's define, which many project use to set for example |
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) |
BabylonJS also has a similiar approach: https://doc.babylonjs.com/typedoc/classes/BABYLON.NullEngine |
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 🙏 |
🙏 |
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). |
So my overall thinking about what we should do is: Use case:
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:
Other considerations:
|
Any updates on this PR ? Because I would have some use for it :) |
This PR aims at uncoupling all core runtime classes of GDJS from their renderers. A few things have been done to that effect:
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
)updatePreRender
,updatePostRender
,render
...) are disabledsetImmediate
/setTimeout
based game loop. Since there is no rendering when that happens,setImmediate
/setTimeout
is a better match thanrequestAnimationFrame
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), howevergetResourceBaseDimensions
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
.