4ian / GDevelop

🎮 Open-source, cross-platform 2D/3D/multiplayer game engine designed for everyone.
https://gdevelop.io
Other
11.49k stars 878 forks source link

Two audio files with the same name, in different folders, the second added doesnt work on playtest #720

Closed blurymind closed 6 years ago

blurymind commented 6 years ago

Describe the bug

This seems to be an obvious issue with audio files specifically - but could affect other file types. I am attaching an example project with the issue.

It happens when two files with the same name exist in different folders, their resource names are different in the project (resource names are relative paths) (same file names, different paths, different resource names)

When you apply the sound file in the event sheet, the second one does not play during playtest.

Some times (rarely) the issue doesnt happen for some reason

To Reproduce

  1. add a resource jump.wav, resulting in name 'jump.wav' (try with jsfx) playtest (it works)
  2. add a resource in subfolder player/jump.wav resulting in name 'player/jump.wav' playtest (sound doesn't play at all)
  3. go to the resource manager and remove jump.wav playtest (player/jump.wav starts working)

If it doesnt initially happen, try changing in the event sheet between the two files by navigating to them

or Open the attached project, playtest (jumping sound doesnt play) twoFilesSameNameBugExample.zip

remove the other audio file with the same file name from resource manager, playtest (jumping sound plays)

Further inspecting this when playtesting , the console shows no error message related to the sound (ctrl+shift+i), when you look at the sources tab - you can see that neither of the jump.wav files were added

This occurs on the latest released version too actually: https://github.com/4ian/GDevelop/releases/tag/v5.0.0-beta55

but it can be in all prvious versions (haven't tested)

blurymind commented 6 years ago

when opening project.json, under resources.resources you can find:

    {
        "file": "player/jump.wav",
        "kind": "audio",
        "metadata": "{\"jsfx\":{\"Frequency\":{\"ChangeAmount\":11.821468336597748,\"ChangeSpeed\":0.15056086396634086,\"Start\":1401.9673578036013},\"Volume\":{\"Decay\":0.3915226459213105,\"Punch\":0.4478189225948841,\"Sustain\":0.05756267729952946}}}",
        "name": "player\\jump.wav",
        "userAdded": true
      },
      {
        "file": "jump.wav",
        "kind": "audio",
        "metadata": "{\"jsfx\":{\"Frequency\":{\"ChangeAmount\":11.821468336597748,\"ChangeSpeed\":0.15056086396634086,\"Start\":1401.9673578036013},\"Volume\":{\"Decay\":0.3915226459213105,\"Punch\":0.4478189225948841,\"Sustain\":0.05756267729952946}}}",
        "name": "jump.wav",
        "userAdded": true
      }

It seems validly saved, so the bug could be somewhere in copying the files before playtest?

4ian commented 6 years ago

Found the issue by using a breakpoint in the function launching the sound (I previously created a sound using jfxr, then another in a subdirectory with the same filename).

You can spot the issue in this screenshot. The resource is here, but the filename ends with ".wav2" as an extension.

image

Here is what happen: 1) Export of resources in GDevelop is pretty simple: the whole resources are copied in the export directory (+ objects and events are scanned in case they also expose ad-hoc resources). 2) During the export, all resources are copied in the export directory with the same filename they had before. 3) In case there is a conflict, like two files with the same filename but in different folder, the second one is renamed to add 2 (or 3, 4, 5...) at the end of its complete name (i.e: at the end of its extension). 4) That's all.

This works well for images, because the loading is properly done even if extension changes it seems. But for audio the sound manager is unable to launch the sound (if I'm not mistaken) when extension is not recognized.

Fix should be done here: https://github.com/4ian/GDevelop/blob/master/Core/GDCore/IDE/Project/ResourcesMergingHelper.cpp#L53

ResourcesMergingHelper::SetFilename should be a bit smarter when giving new filenames. Will try to fix that, unless someone want to give it a try. Have to be sure that it's not breaking export.

It seems validly saved, so the bug could be somewhere in copying the files before playtest?

Exactly, you had the right intuition here ;) Chrome Debugger is helpful in these situations to dig more into the issue.

blurymind commented 6 years ago

@4ian thank you for looking into this

4ian commented 6 years ago

Should be fixed in https://github.com/4ian/GDevelop/commit/569adc150047dc4cd3ea5d11510c4cae89d15bd9 :)

Files (in different folders or not) but with the same filename will be copied during export but renamed properly (extension will stay the same, only the base name will be updated to be unique).

Just updated libGD.js with this, as usual remove public/libGD.js and relaunch yarn start/npm start.

Let me know if this works so that we can close this :)

4ian commented 6 years ago

(Updated libGD.js now)

blurymind commented 6 years ago

That was fast!! Thank you @4ian 😄

4ian commented 6 years ago

Let's close this if you have time to confirm that this is working better :) I've tested with jfxr and it seems ok now :)

blurymind commented 6 years ago

@4ian sorry it takes me a while to test this one. Yes please close it :) Thank you again for the fix