Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.3.0 project cannot be opened in 4.0.2 #507

Closed
StepanMynarik opened this issue Mar 27, 2024 · 8 comments · Fixed by #532
Closed

3.3.0 project cannot be opened in 4.0.2 #507

StepanMynarik opened this issue Mar 27, 2024 · 8 comments · Fixed by #532
Assignees
Labels
type:bug Something isn't working

Comments

@StepanMynarik
Copy link

Describe the bug
Hi,

I have customized the base template project with my son, initially on 3.2.0, then updated to 3.3.0 with no difficulty.

Today I updated to 4.0.2 but I can't open the project in this version, getting TypeError: Cannot get properties of undefined (reading 'length') and I simply stay on the start screen then.

To Reproduce
Steps to reproduce the behavior:

  1. Download the attached project
  2. Open "zip" (portable) version of ct.js 4.0.2
  3. Try opening the project from the start screen
  4. See error

Expected behavior
Project will be opened. It will be possible to start the game and hear sounds when jumping etc.

System information report
Ct.js v4.0.2 😽 (packaged)

NW.JS v0.72.0
Chromium v109.0.5414.87
Node.js v19.3.0
Pixi.js v7.3.2

WebGPU UNAVAILABLE
WebGL available
WebGL vendor Google Inc. (NVIDIA)
WebGL renderer ANGLE (NVIDIA, NVIDIA GeForce RTX 3060 Ti Direct3D11 vs_5_0 ps_5_0, D3D11)
OS win32 x64 // Windows_NT 10.0.22631

Screenshots (recommended)
image

Example project (strongly recommended)
Bertas First Game.zip

@StepanMynarik StepanMynarik added the type:bug Something isn't working label Mar 27, 2024
@CosmoMyzrailGorynych CosmoMyzrailGorynych self-assigned this Mar 28, 2024
Copy link

stale bot commented May 27, 2024

Hey there 👋 I marked this issue as stale as it hadn't brought much attention for quite a while.
I do understand that stale issues are still issues, yet here stale issues receive the least attention from maintainers so they can focus on more relevant tasks. You can help with this issue by checking whether it affects latest versions of ct.js and writing about it if it does, providing an example project and steps to reproduce. Or maybe you can close it with a PR!
Note that some platform-dependent issues can't be resolved by developers due to the absense of such devices :c We will need help from you for such tasks.
If this issue won't get attention in three months, it will be closed.

@stale stale bot added the state:stale This aged well. Probably not relevant now. label May 27, 2024
@godmar
Copy link
Contributor

godmar commented Jul 10, 2024

This is likely due to changes in the sound entries.
The code appears to assume that these entries contain a .variants property that is an array. This property was not present in 3.x before the change to the new sound engine.

See, for instance, here and here which contains expressions such as sound.variants.length or sound.variants.push.

@CosmoMyzrailGorynych could you fix this or advise?

We won't be able to continue to use ct.js if we can't import our 3.x project.

@stale stale bot removed the state:stale This aged well. Probably not relevant now. label Jul 10, 2024
@godmar
Copy link
Contributor

godmar commented Jul 11, 2024

Could you take a look at your project migration scripts?

You're calling createAsset() here.

I see two problems here:

  1. you're calling it without a name - that causes it to put up a prompt to the user which appears random.
  2. it returns a Promise, but you don't await it. So you treat newSound as though it were an object rather than a Promise. (I don't know why createAsset is async in the first place - does it have to be?)

Consider this patch:

--- a/src/js/projectMigrationScripts/4.0.0-next-3.js
+++ b/src/js/projectMigrationScripts/4.0.0-next-3.js
@@ -47,15 +47,15 @@ window.migrationProcess.push({
                     delete sound[oldKey];
                 }
 
-                const newSound = createAsset();
-                delete newSound.name;
-                delete newSound.uid;
-                newSound.preload = preload;
+                async.push(createAsset(sound.name).then((newSound => {
+                    delete newSound.name;
+                    delete newSound.uid;
+                    newSound.preload = preload;
 
-                Object.assign(sound, newSound);
+                    Object.assign(sound, newSound);
 
-                async.push(addSoundFile(sound, oldFile)
-                    .then(() => fs.remove(oldFile)));
+                    return addSoundFile(sound, oldFile).then(() => fs.remove(oldFile));
+                })));
             }
         }
         if (async.length > 0) {

Alternatively, you could make createAsset synchronous if possible.

@CosmoMyzrailGorynych
Copy link
Collaborator

CosmoMyzrailGorynych commented Jul 11, 2024

I don't know why createAsset is async in the first place - does it have to be?

Yes, precisely for when the IDE needs to prompt the user for a name, plus for other async tasks like copying or converting files. The common resources API requires all asset types' modules to expose a specific interface, so all asset creations tasks are awaited.

@CosmoMyzrailGorynych
Copy link
Collaborator

@godmar I've pushed a fix to the develop branch, please test the upcoming Nightly (or run from sources) to confirm/deny that the problem is solved.

@godmar
Copy link
Contributor

godmar commented Jul 11, 2024

I'm testing this now; meanwhile, another error pops up for the Berta....zip file @StepanMynarik provided: ...mouse/module.json not found. In commit 7db23c5 you removed ct.mouse but I don't see where this is accounted for in the migration scripts. Do you need this:

--- a/src/js/projectMigrationScripts/4.0.0-next-1.js
+++ b/src/js/projectMigrationScripts/4.0.0-next-1.js
@@ -12,6 +12,7 @@ window.migrationProcess.push({
         }
         delete project.libs.fittoscreen;
         delete project.libs.touch;
+        delete project.libs.mouse;
         delete project.libs['sound.howler'];
         delete project.libs['sound.basic'];
 

@godmar
Copy link
Contributor

godmar commented Jul 11, 2024

For my test game, the sound conversion aspect now appears to work, but the game itself does not. I'll show you on Discord.

PS: this issue turned out to be fixed as advised by disabling Vulkan and restarting ct.js

@godmar
Copy link
Contributor

godmar commented Jul 13, 2024

More observations. After commit 1de688d the game can now be imported, but there's a semantic change that causes the existing code to fail when it comes to using the sounds API.

Sounds that were marked as "background music" (isMusic == true) now have the preload flag set, whereas those that are not are not preloaded. In the new sound implementation, it appears that it's illegal to call methods such as sounds.playing or sounds.stop on sounds that haven't been preloaded or loaded. Sounds are loaded when they are first played, or when sound.load is called.

This is a breaking change - the 3.x games worked fine when these methods were called on sounds that hadn't been loaded. I believe that perhaps the semantics of these sounds functions could be changed to accommodate not-yet-loaded sounds. (Also in light of sounds.exists, which returns true for sounds even if they haven't been loaded.) In any event, it may be helpful to document this issue in the migration guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants