[Community] [InkJS] Dialogue tree with InkJS

[infokub]

Edit gdjs authorization for InkJS extension and add Extension file

[infokub]

Demo project : https://gd.games/infokubarcade/ink-demo
The story is written in Ink and read by the extension.
The other content (text appearance, choice appearance, backgrounds etc.) are made with regular GDevelop events.

[D8H]

Thank you for submitting an extension.

A few things before starting the review:

• The demo must be a project because it allows reviewers to understand how the extension is used.
• The library should not be minified to let reviewers do a quick check for ill intended code.
• The library version and a link to the repository should be added in a comment to allow extension maintainers to easily update it.

[4ian]

Hey - we would love to have this in the extension list. Is there any blocker or things we can help on, on the 3 points of the list written by @D8H?
Happy to discuss!

[infokub]

Hey, the only blocker is me not checking my Github account 😐

I'll do this right away so it will be ready for tomorrow morning 👌

[infokub]

Maintenance

Ink source : https://github.com/inkle/ink
InkJS : https://github.com/y-lohse/inkjs

Current support :
Ink 1.1.1
InkJS 2.2.0

Not supported :
Multiple flows (beta on Ink side)

[infokub]

Demo project : https://drive.google.com/drive/folders/1ttw6_2M3QryKNYx0pwtrZmQtJswevG9U?usp=sharing

[infokub]

For now i don't know how to not use the minified version since it's the only accessible file i know.
So let me know what i have to do to build a not minified file and i'll push it tomorrow.

[D8H]

For now i don't know how to not use the minified version since it's the only accessible file i know. So let me know what i have to do to build a not minified file and i'll push it tomorrow.

The library uses the Terser plugin of Rollup. I guess you can get a not minified version by removing these lines: terser(),
https://github.com/y-lohse/inkjs/blob/master/rollup.config.js#L2C10-L2C17

[D8H]

Thank you for submitting an extension.

It looks great already. I have a few questions and suggestion:

• Your example is great to show that the extension is actually easy to use. As the extension has a lot of features, it will probably frighten users if they can't rely on an example. Do you think it could be submitted too?
  • You can create a fork of the example repository: https://github.com/GDevelopApp/GDevelop-examples
  • It will allow you to give a link to open your example directly in GDevelop online editor https://editor.gdevelop.io/?project=https://raw.githubusercontent.com/infokub/GDevelop-examples/my-branch/examples/my-example/my-example.json
  • This way you won't have to upload the project archive nor push the extension at every back and forth (we can update the extension branch only once right before the merge). You'll only have to push the example changes.
• When does a game need several stories?
• Having to register variables for the VariableChanged condition to work doesn’t' sound user-friendly. Do you think the condition could automatically register the variable observer if it doesn't exist yet? In this case, the condition could return true as users may want to handle the initial value.
• About ObserveAndConnectVariable
  • Does it work both ways?
  • What happens after a scene change? As it seems that the extension keeps a variable instance that will no longer exist.
  • I think it may be better not to keep this action.
• ChangeVariableBoolean is missing a function group
• "n°" is French for "#" but I guess just the number is better.
• Classes don't need to be affected to a local variable. They can be used directly as in this documentation: https://wiki.gdevelop.io/gdevelop5/events/js-code/javascript-in-extensions/#embed-javascript-libraries-in-extensions
• GDevelop JS editor understands JSDoc. It will give you better autocompletion.
  https://jsdoc.app/about-getting-started
• The library should not be defined globally as it could conflict with other extensions. You can remove the library package code and only keep the classes. If you put these classes in the same JS event as your own class, you'll even have autocompletion.

[infokub]

* Your example is great to show that the extension is actually easy to use. As the extension has a lot of features, it will probably frighten users if they can't rely on an example. Do you think it could be submitted too?

I could yes of course. I also know that EchoDangler is creating a template around it already.

* When does a game need several stories?

If we are talking about a classic VN. Never.
But if you are making a Quest System you could require to split that into multiple files (one per place for example) and not use the automatic flow of Ink.
I gave this possibility for not adding constraints for devs. But the counterpart is to write the Story ID everytime.
Maybe having a Default ID when it's blank ? Like Layers ?

* Having to register variables for the `VariableChanged` condition to work doesn’t' sound user-friendly. Do you think the condition could automatically register the variable observer if it doesn't exist yet? In this case, the condition could return true as users may want to handle the initial value.

I see what you mean, It's what you need to do with any other languages. I could make this auto yes. Just to be clear : the initial value is already set inside Ink, that's why there is no "Change" event thrown here.

* About `ObserveAndConnectVariable`
  
  * Does it work both ways?

It does it one way and a half.
In fact, you can't edit Ink stuff only using the internal "Change Variable" GDevelop obviously.
But if you use the Change Variable of Ink, it will also cycle through all the observers as well, and edit the Variable since you requested it with this Action.

  * What happens after a scene change? As it seems that the extension keeps a variable instance that will no longer exist.

Absolutely not handheld. You are right.

  * I think it may be better not to keep this action.

It's up to GDevelop.

* `ChangeVariableBoolean` is missing a function group
* "n°" is French for "#" but I guess just the number is better.

Noted.

* Classes don't need to be affected to a local variable. They can be used directly as in this documentation: https://wiki.gdevelop.io/gdevelop5/events/js-code/javascript-in-extensions/#embed-javascript-libraries-in-extensions

* GDevelop JS editor understands JSDoc. It will give you better autocompletion.
  https://jsdoc.app/about-getting-started

* The library should not be defined globally as it could conflict with other extensions. You can remove the library package code and only keep the classes. If you put these classes in the same JS event as your own class, you'll even have autocompletion.

I'll look forward this. It will require for me to re-write the entire extension though.

Side notes :

• bug in Change Variable Number (return stuff instead of changing it)

[D8H]

• The library should not be defined globally as it could conflict with other extensions. You can remove the library package code and only keep the classes. If you put these classes in the same JS event as your own class, you'll even have autocompletion.

I'll look forward this. It will require for me to re-write the entire extension though.

To save time, you can do a search and replace into the extension JSON file. But, it seems there is only one occurrence of inkjs. (var story = new inkjs.Story(json);) so it should actually be faster manually (to use gdjs._Ink.Story after moving the lib to a local declaration).

[4ian]

I wonder if this shows that we should add an easier way to include entire JS files (and .d.ts TS definitions) to extensions.

[D8H]

Please attach a new version of the demo. It will help with the review process.

[infokub]

New demo
Inky_1.1.0.zip

[D8H]

LGTM, Thanks a lot! It should be very useful.