Skip to content

Conversation

JamesVanBoxtel
Copy link
Contributor

This PR adds animation support to the game via customizable JSON files.

In the files you specify a hierarchy of images and animations that are loaded into what I'm calling drawable objects for now.
We probably will want to roll these objects into UIElements in the future.

Animations are setup to update with the flux engine and cleanup themselves if they finish. If they loop forever they must be removed manually. We do this on a new sceneDidDisappear call.

Full documentation is at docs/animations.md and unit tests are also included.

Here is an example movie of what I've created with this so far to demonstrate what's possible. Will be reaching out to the community for feedback on the city as it will need iteration.
https://github.com/user-attachments/assets/d5f7c704-4968-4050-8242-5b74df6f8e18

Copy link
Collaborator

@Endaris Endaris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial feedback, I didn't look at everything.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For single file libs we should include the license at the top of the file.

CustomRun.runTimeGraph = nil
end

Flux.update(dt)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should update flux in love.update.
Doing so means that all looping tweens of scenes in the call stack will keep updating even though only the top scene actually is actually being rendered.
We should make use of flux's group feature and have each scene call update(dt) for its own flux group.

Comment on lines +15 to +131
{
"id": "backgroundBottomLeftGear",
"anchor": "topLeft",
"position": {
"x": 98,
"y": 240
},
"children": [
{
"filePath": "Gear0",
"anchor": "topLeft",
"position": {
"x": 0,
"y": 0
},
"animationTracks": [
{
"loopTrack": true,
"steps": [
{
"animateProperties": {
"rotation": 6.28318
},
"durationSeconds": 20,
"easeType": "linear"
}
]
}
]
},
{
"filePath": "Gear1",
"anchor": "topLeft",
"position": {
"x": 0,
"y": 0
},
"animationTracks": [
{
"loopTrack": true,
"steps": [
{
"animateProperties": {
"rotation": 6.28318
},
"durationSeconds": 20,
"easeType": "linear"
}
]
}
]
},
{
"filePath": "Gear2",
"anchor": "topLeft",
"position": {
"x": 0,
"y": 0
},
"tint": [
1,
0,
0
],
"animationTracks": [
{
"loopTrack": true,
"steps": [
{
"animateProperties": {
"rotation": 6.28318
},
"durationSeconds": 20,
"easeType": "linear"
}
]
}
]
},
{
"filePath": "Gear3",
"anchor": "topLeft",
"position": {
"x": 0,
"y": 0
},
"yoyo": true,
"animationTracks": [
{
"yoyo": true,
"steps": [
{
"animateProperties": {
"alpha": 0.8,
"xScale": 1.2,
"yScale": 1.2
},
"durationSeconds": 1,
"easeType": "quadinout"
}
]
}
]
},
{
"filePath": "Gear4",
"anchor": "topLeft",
"stencil": true,
"alpha": 0.3,
"blendMode": "add",
"position": {
"x": 0,
"y": 0
}
}
]
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like this kind of composite behaviour being fully baked into our structure the way it is.
The disadvantage this has is that it breaks love's auto batching by design.
In a scenario with many gears, drawing gear0 first for all of them, gear1 second etc. will use auto batching. Given the likely use case that people will use scripting languages for more complex use cases to generate the script output I think that keeping the door for auto batching open is the correct move at this point in time.
The current main menu scene already takes 0.5ms on my new machine. It's practically inevitable people that people will try to push the animation system much further than you did on the main menu with the wide amount of features you added.

I think the advantage of allowing relative positioning to the parent is not as important as it is in UI as for composed elements like this one there is relatively little harm done in adding extra transparency around the image and making all of them the same size (or in a rotation symmetrical case like here, just anchoring them all at center) to get a similar ease of use where you can simply draw all images in a set to the same coordinate.

Instead we could also consider offering a way to organize sprite batches but I feel like that is not necessarily going to work well in practice as spritebatches used incorrectly also perform worse than the autobatcher. That is something that we could maybe expand on later but I don't see it in an initial version.

We could also try to automatically generate atlases and draw from them instead of the images...but that also has its limits and will probably generate a lot of extra work because we're working with arbitrary textures on arbitrary systems so there is a bunch of things we'd have to look out, see https://www.love2d.org/wiki/GraphicsFeature https://www.love2d.org/wiki/GraphicsLimit
With Android on board that does not really inspire a lot of enthusiasm so personally I'd keep it on the simple side for now.

Comment on lines 423 to 431
love.graphics.setBlendMode(d.blendMode, d.alphaMode)
love.graphics.setColor(d.tint[1], d.tint[2], d.tint[3], d.alpha)
if d.quad then
d.quad:setViewport(d.scrollX, d.scrollY, d.width, d.height)
love.graphics.draw(d.texture, d.quad, 0, 0)
else
love.graphics.draw(d.texture, 0, 0)
end
love.graphics.setStencilTest()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setStencilTest and setBlendMode both break love's autobatching so they should only be called if it's actually necessary.
Given the setup only the stars significantly benefit but just checking against d.stencil and the output of getBlendMode is already 20% less draw time for the main menu.

@JamesVanBoxtel JamesVanBoxtel marked this pull request as draft September 28, 2025 16:42
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.

2 participants