Skip to content

feat(ui): introduce UIAnimatedImage for animated sprites and use in ship preview - #724

Open
BenjaminAmos wants to merge 1 commit into
MovingBlocks:developfrom
BenjaminAmos:ui-animated-sprites
Open

feat(ui): introduce UIAnimatedImage for animated sprites and use in ship preview#724
BenjaminAmos wants to merge 1 commit into
MovingBlocks:developfrom
BenjaminAmos:ui-animated-sprites

Conversation

@BenjaminAmos

Copy link
Copy Markdown
Contributor

Description

This pull request introduces a new UI widget, called UIAnimatedImage, that allows animated images to be rendered within NUI UI screens. The API is similar to that of UIImage, however, the image binding has been replaced with three separate bindings:
- spritesheet (UITextureRegion) - The spritesheet to use for rendering.
- frameDuration (float) - The number of seconds to elapse between each frame shown
- frames (List<Rectanglei>) - The pixel co-ordinates of each individual frame.

The new UI widget is used to render ship previews on the main menu, enabling support for animated ship sprites to be correctly displayed.

This pull request also changes the rendering logic for the main menu background ships to correctly render animated ship sprites.

Testing

Follow the instructions #723 and verify that the expected behaviour is seen.

Notes

@Du4lity5151

Copy link
Copy Markdown
Contributor

This fix appears to work mostly correctly, however the animation plays slower in the background if I add the imperialBigElite as a background ship in menuBackgroundShipConfig. If you notice flickering in the siren lights themselves, then it might be normal as the lights flash for one frame and then go off for the next two frames, and repeats at 48 FPS. Otherwise, I did not notice any other issues.

@NicholasBatesNZ NicholasBatesNZ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Really nice addition — a reusable UIAnimatedImage is exactly the right shape for this, and the spritesheet slicing math checks out (the getRegionY() - getRegionHeight() bit looks odd at first but is correct given the flip(false, true) in Assets.getAnimation). Two things I'd like to see before merging, though.

First, I think @Du4lity5151's slowdown report is a genuine bug rather than a quirk of that sprite: SolApplication.render() drains its accumulator in fixed Const.REAL_TIME_STEP (1/60) chunks, but NUIManager hands each screen Gdx.graphics.getDeltaTime() — so with the desktop default of 100 target FPS and vsync off, MenuBackgroundObject.animationTime only advances 60 × 0.01 = 0.6s per real second, i.e. everything plays at 60% speed (and would run fast below 60 FPS). In-game sprites use game.getTimeStep() and are unaffected, which is why the background looks out of step. Passing Const.REAL_TIME_STEP should sort it.

Second, UIAnimatedImage.onDraw has if (spritesheet.get() != null || frames.get().isEmpty()) where getPreferredContentSize correctly uses && !frames.get().isEmpty() — as written an empty frame list walks straight into frames.get().get(frameNo). Related: NewShipScreen still adds null to playerSpawnConfigTextures in the catch block, but the new code does .get(index).texture, so a ship whose texture fails to load now NPEs instead of quietly rendering nothing.

Also worth frameTimer -= frameDuration rather than = 0 in update so the animation doesn't get quantized to the update tick. Happy to re-review once those are in!

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.

Spritesheets are displayed instead of animations if not in-game

3 participants