search

Scirra / Construct-bugs

Public bug report submissions for Construct 3 and Construct Animate. Please read the guidelines then click the 'Issues' tab to get started.
https://www.construct.net
107 stars 83 forks source link

Copying/pasting a folder in animation editor causes duplicate "sid" property #6125

Closed dop2000 closed 2 years ago

dop2000 commented 2 years ago

This is a pretty serious bug as it can corrupt objects in the project and people may not be aware of this.

Problem description

I noticed a weird issue with sprite growing in size by itself when pasted on drawing canvas, eventually reaching infinity. This only happened with some animations in the sprite, added recently. (there are over 340 animations in the sprite) Turned out some animations have the same "sid" property in JSON.

image

Attach a .c3p

https://www.dropbox.com/s/7n32dyqs07sm9ov/sid-bug.c3p?dl=0

Steps to reproduce

  1. Click multiple times, notice how the itemIcon sprite changes animation automatically and grows in size. This is because crescentplantCrop and crescentplantSeeds animations have the same "sid"

I was also able to reproduce this in the red Sprite, by copying the animation folder and pasting. I'm worried this may happen with other operations, like importing animations from zip.

Could you advise the best way of fixing the sprite - to make all "sids" unique? I dread the idea of re-importing all 340 animations...

More details

The bug is in the latest stable version. I tried the latest beta, it has the same problem. However copying/pasting a folder there also crashes the editor, I'll post a new bug report.

System details

View details Platform information Product: Construct 3 r308.2 (stable) Browser: Chrome 105.0.5195.127 Browser engine: Chromium Context: webapp Operating system: Windows 10 Device type: desktop Device pixel ratio: 1.75 Logical CPU cores: 16 Approx. device memory: 8 GB User agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/105.0.0.0 Safari/537.36 Language setting: en-US Local storage Storage quota (approx): 275 gb Storage usage (approx): 528 mb (0.2%) Persistant storage: No Browser support notes This list contains missing features that are not required, but could improve performance or user experience if supported. Nothing is missing. Everything is OK! WebGL information Version string: WebGL 2.0 (OpenGL ES 3.0 Chromium) Numeric version: 2 Supports NPOT textures: yes Supports GPU profiling: yes Supports highp precision: yes Vendor: Google Inc. (AMD) Renderer: ANGLE (AMD, AMD Radeon(TM) Graphics Direct3D11 vs_5_0 ps_5_0, D3D11) Major performance caveat: no Maximum texture size: 16384 Point size range: 1 to 1024 Extensions: EXT_color_buffer_float EXT_color_buffer_half_float EXT_disjoint_timer_query_webgl2 EXT_float_blend EXT_texture_compression_bptc EXT_texture_compression_rgtc EXT_texture_filter_anisotropic EXT_texture_norm16 KHR_parallel_shader_compile OES_draw_buffers_indexed OES_texture_float_linear WEBGL_compressed_texture_s3tc WEBGL_compressed_texture_s3tc_srgb WEBGL_debug_renderer_info WEBGL_debug_shaders WEBGL_lose_context WEBGL_multi_draw OVR_multiview2 Audio information System sample rate: 48000 Hz Output channels: 2 Output interpretation: speakers Supported decode formats: WebM Opus (audio/webm; codecs=opus) Ogg Opus (audio/ogg; codecs=opus) WebM Vorbis (audio/webm; codecs=vorbis) Ogg Vorbis (audio/ogg; codecs=vorbis) MPEG-4 AAC (audio/mp4; codecs=mp4a.40.5) MP3 (audio/mpeg) FLAC (audio/flac) PCM WAV (audio/wav; codecs=1) Supported encode formats: WebM Opus (audio/webm; codecs=opus) Video information Supported decode formats: WebM AV1 (video/webm; codecs=av01.0.00M.08) MP4 AV1 (video/mp4; codecs=av01.0.00M.08) WebM VP9 (video/webm; codecs=vp9) WebM VP8 (video/webm; codecs=vp8) Ogg Theora (video/ogg; codecs=theora) H.265 (video/mp4; codecs=hev1.1.2.L93.B0) H.264 (video/mp4; codecs=avc1.42E01E) Supported encode formats: WebM VP9 (video/webm; codecs=vp9) WebM VP8 (video/webm; codecs=vp8)
AshleyScirra commented 2 years ago

@DiegoScirra - copy/pasting needs to set the "copy" mode when saving to JSON to avoid copying the SID.

dop2000 commented 2 years ago

@AshleyScirra @DiegoScirra I'm worried there may be other sprites with duplicated SID in animations, which may cause issues in the future. Is there any way to find them or re-index all SIDs in the project?

DiegoScirra commented 2 years ago

This will be fixed in the next beta. Was a regression introduced in r306.

@AshleyScirra Should we do anything about existing projects that have been affected by this?

AshleyScirra commented 2 years ago

I had a look at where animation SIDs are used and I believe they're only used by the savegame system (SID stands for Save ID after all). Loading a savegame may restore the wrong animation if the project includes duplicate animation SIDs, but that seems fairly low risk - it's certainly not anything like project corruption. That also can't be directly fixed as the savegame data itself is ambiguous if saved from a project with duplicate SIDs.

Editing the project data directly to change any duplicate SIDs should work around this - SIDs are just random numbers so as long as they're all different it will be OK.

I can't open the provided project as it uses a third-party addon (GridView Data Bind) - unless the project uses savegames, I'm sceptical the issue with SIDs is related to the described issue about a sprite getting larger, but DrawingCanvas does save and restore state so it might be related still.

dop2000 commented 2 years ago

@AshleyScirra I'm sorry, I re-uploaded the project without the behavior: https://www.dropbox.com/s/7n32dyqs07sm9ov/sid-bug.c3p?dl=0 I tried changing the SID manually and it did fix the issue with the sprite getting larger, so it's definitely related.

dop2000 commented 2 years ago

@AshleyScirra @DiegoScirra Could you advise if duplicate SIDs in two different sprites can cause any issues? I discovered that because we copied animations between sprites, the same SIDs can now be found in multiple sprites.

AshleyScirra commented 2 years ago

Could you advise if duplicate SIDs in two different sprites can cause any issues?

I already did:

... I believe they're only used by the savegame system (SID stands for Save ID after all). Loading a savegame may restore the wrong animation if the project includes duplicate animation SIDs, but that seems fairly low risk - it's certainly not anything like project corruption.

dop2000 commented 2 years ago

... I believe they're only used by the savegame system (SID stands for Save ID after all). Loading a savegame may restore the wrong animation if the project includes duplicate animation SIDs

Does this apply only to the situation when there are duplicate SIDs within the same sprite? What I'm asking is can anything bad happen when two animations in two different sprites share the same SID.

AshleyScirra commented 2 years ago

I don't think duplicate SIDs in different sprites will be a problem at all. Sprites only look up their own animations by SID, so the SIDs only need to be unique across the sprite's own animations.

AshleyScirra commented 2 years ago

This should now be fixed in r312, so that copy/pasting animations no longer duplicates SIDs.

I'm not confident about taking further action to try to renumber SIDs in projects where this has already happened though. It's pretty complex code and I think there's quite a high risk of regression, and then in theory that compatibility code has to stay there forever just in case someone opens one of those projects. Given the relatively low severity of this issue (probably affects savegames sometimes; unlikely to have any more serious consequence; can be worked around by editing SIDs in project file if really necessary) I don't think we'll take any further action on that at this time, so closing this issue.