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

Can't change voyager-story's mode after initialization #246

Closed
sdumetz opened this issue Jan 17, 2024 · 3 comments
Closed

Can't change voyager-story's mode after initialization #246

sdumetz opened this issue Jan 17, 2024 · 3 comments

Comments

@sdumetz
Copy link
Contributor

sdumetz commented Jan 17, 2024

If I try to call taskProvider.ins.mode.setValue(3) - or whatever number, after voyager story has initialized, everything breaks.

The funny thing is: it breaks even before CVTaskManager.update() gets called.

I looked around a bit and it might not be easy to make this reentrant. Maybe the easiest fix would be to prevent modification like this:

--- a/source/client/components/CVTaskProvider.ts
+++ b/source/client/components/CVTaskProvider.ts
@@ -72,10 +72,12 @@ export default class CVTaskProvider extends CComponentProvider<CVTask>
         const languageManager = this.languageManager;
 
         if (ins.mode.changed) {
-            this.scopedComponents.forEach(task => task.dispose());
-
-            const taskSet = taskSets[ins.mode.getValidatedValue()];
-            taskSet.forEach(taskType => this.createComponent(taskType));
+            if(this.scopedComponents.length) {
+                console.error("Can't change CVTaskProvider mode after initialization");
+            }else{
+                const taskSet = taskSets[ins.mode.getValidatedValue()];
+                taskSet.forEach(taskType => this.createComponent(taskType));
+            }
         }

Or there is an easy(ish) fix that I missed? I'm not yet familiar enough with this part of the code.

@gjcope
Copy link
Collaborator

gjcope commented Jan 17, 2024

I will take a look. It looks like it was intended to be reentrant (I did not write this code) but I also don't have a good use case for updating mode at runtime so your proposal may be fine.

@gjcope
Copy link
Collaborator

gjcope commented Jan 22, 2024

@sdumetz if you make the small bugfix to CaptionView here (2a57d75)

and remove the 2 lines here:

this.application.dispose();

it should work.

The issue is that I think this will break the app memory being released in the case where someone destroys the component element in the page. I.e., recreating to change scene rather than using the app reload functionality.

@gjcope
Copy link
Collaborator

gjcope commented Jan 25, 2024

Fixed in v0.37.0

@gjcope gjcope closed this as completed Jan 25, 2024
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

No branches or pull requests

2 participants