feat(ui): introduce UIAnimatedImage for animated sprites and use in ship preview - #724
feat(ui): introduce UIAnimatedImage for animated sprites and use in ship preview#724BenjaminAmos wants to merge 1 commit into
Conversation
|
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
left a comment
There was a problem hiding this comment.
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!
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 ofUIImage, however, theimagebinding 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
imperialBigEliteship from theeliteVangardmodule. It is unclear whether this is a spritesheet design or renderer issue.