blurymind / YarnClassic

A tool for writing interactive dialogue in games!
MIT License
518 stars 51 forks source link

[Discussion] Process to update Yarn Editor to the Yarn 2.0 spec? #236

Closed FaultyFunctions closed 3 years ago

FaultyFunctions commented 3 years ago

Hello! I was wondering if we had any thoughts on when we should try to tackle this and how?

Just for reference: https://gist.github.com/McJones/d9a9570cd4dcbb23746c23903d8bfe59

and: https://github.com/YarnSpinnerTool/YarnSpinner/pull/285

blurymind commented 3 years ago

Well there are two ways to do it imo:

Pr to bondagejs to add the spec features Or Port yarnspinner to js

First one seem lower hanging fruit, but needs some feedback from @hylyh imo

On Mon, 18 Jan 2021, 00:03 Faulty, notifications@github.com wrote:

Hello! I was wondering if we had any thoughts on when we should try to tackle this and how?

Just for reference: https://gist.github.com/McJones/d9a9570cd4dcbb23746c23903d8bfe59

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/YarnSpinnerTool/YarnEditor/issues/236, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRRWVII6WZSAQJNNJYLRSDS2N3E3ANCNFSM4WGQLNWQ .

FaultyFunctions commented 3 years ago

Alrighty I was just wondering, tbh the bondagejs stuff might be over my head. I might be able to work on the editor stuff like changes to highlighting in the editor for the new spec if need be.

JujuAdams commented 3 years ago

What's bondagejs used for, just the playback right? If this is the case, it would make sense to begin work on the rest of the editor features in accordance with the v2 spec since playback is dependent on the Yarn script and not the other way round

FaultyFunctions commented 3 years ago

I took a look at the code and it seems to me the bondage.js is only being used for the playback preview. Please correct me if I'm wrong. I think we should start work on getting up to spec like Juju said. I think we should focus on the Yarn filetype as the primary goal and the JSON filetype as a close secondary goal. (even though the 2.0 spec doesn't call for JSON support, I still think we should implement it after we get the Yarn filetype up and running.) We should ignore all the unnecessary stuff for now and just focus on implementing the new spec.

blurymind commented 3 years ago

What's bondagejs used for, just the playback right? If this is the case, it would make sense to begin work on the rest of the editor features in accordance with the v2 spec since playback is dependent on the Yarn script and not the other way round

bondagejs is used by a number of html5 game engines (ct.js, gdevelop and so on). I must absolutely stress that we should not treat Unity3d as the only game engine in the world and break functionality in other game engines to please unity users. There are huge communities of users outside of unity using yarneditor now.

The reason I started contributing to yarneditor is gdevelop. Bondagejs is not just for playback- its our compatibility with more game engines.

So no. We must absolutely treat the json file format and ability to run in html5 engines as a first class citizen imo.

I am all for bumping to the yarn v2 spec, but if in the process you create bugs or break those two things, I am going to reject your prs until you fix em :)

@FaultyFunctions can you be a bit more specific about what entails bumping the spec in the editor? This whole post is very vague. What specific features do you want to add or change?

If you guys want to get rid of bondagejs - port yarnspinner to run in the browser, as a javascript module. Publish it on npm - then when we can get spinner to run in yarn editor, we should be able to get it running in gdevelop and other html5 engines. I am surprised nobody tried porting it via wasm and making a wrapper

JujuAdams commented 3 years ago

I am taken aback by the hostility in your tone. Faulty and I are from the GameMaker community, we have spent a lot of time working on getting Yarn v1 up and running in our engine of choice. Please don't insult us by giving a lecture on cross-engine compatibility.

To be absolutely clear - bondagejs is a good thing, compatibility with other engines is a good thing.

Having said that, you will want to finish your conversation with the Yarn team about JSON support because, as far as I can tell, JSON is still officially deprecated. Whether or not supporting JSON is a good idea is something that needs to be sorted out with YarnScript team.

The ultimate purpose of YarnEditor is to provide a comfortable editing experience for YarnScript (creating nodes, inputting text, autocompletion for syntax, file import/export). Playback is a lovely feature to have and a full release should indeed include it. Playback is not, however, a fundamental architectural feature for editing YarnScript. Our (Faulty and I) goal is to bring the script editing features into spec with YarnScript v2. This is a matter of critical path analysis - bondagejs blocks full release but doesn't block preliminary work into the script editing experience.

As people wanting to contribute to YarnEditor, it is entirely unreasonable for us be contrained entirely by progress made by bondagejs. My fear is - one that should be obvious at this point - is that we will not be able to update YarnEditor for YarnScript v2 for some time, and insisting that bondagejs completely supports v2 before we're even "allowed" to touch YarnEditor will massively delay delivery of a v2-compliant YarnEditor, even as a minimum viable product.

Remember - YarnEditor as a product is cross-engine. Stalling development of the editor for the sake of bondagejs is the exact opposite of what you're arguing for.

LunaTheFoxgirl commented 3 years ago

This convinces me that I should just go ahead and write my own yarn editor for my YarnSpinner port.

FaultyFunctions commented 3 years ago

@blurymind You're being completely unreasonable here and contradicting yourself. You're saying YarnEditor needs to remain engine agnostic while also trying to get us to specifically cater to GDevelop which is actively holding back development. Compatibility with the runner will come in due time but we need to get started on bringing the editor up to spec for the new changes.

In regards to what has changed please see: https://github.com/YarnSpinnerTool/YarnSpinner/blob/yarn-spec/Documentation/Yarn-Spec.md and more specifically: https://github.com/YarnSpinnerTool/YarnSpinner/blob/develop/CHANGELOG.md

Also, is the editor not completely useable for the GDevelop community right now? We're not saying they need to jump ship and use a new v2 compliant editor without a similarly compliant runner. They can continue using the current version of YarnEditor and bondage.js, but why hold every other engine community hostage in the process?

So no. We must absolutely treat the json file format and ability to run in html5 engines as a first class citizen imo.

I agree with this point. I was merely stating that JSON be looked at second because the Yarn team has given us specifications in what a 2.0 compliant yarn file looks like. So, it's a good jumping off point to get started. We are not the ones saying drop JSON support, your arguments with that should go towards the Yarn team. I am in favor of keeping JSON support for as long as we can, if not indefinitely.

If you want to continue to hinder development for the sake of catering to GDevelop then we can always branch off and make whatever necessary changes in our own forks or our own tools, just like @LunaTheFoxgirl has said. However I would rather work together within the YarnSpinnerTool system because that's what will be most beneficial for the community (not just GDevelops) as a whole.

shdwcat commented 3 years ago

Maybe I'm oversimplifying, but what if:

Nothing breaks, development doesn't get held back by anything. Worry about releases later?

blurymind commented 3 years ago

I don't mean to be disrespectful, just merely stating that by dropping compatibility with the syntax in bondagejs you are going to make it unusable in gdevelop. Yarn is currently bundled with gdevelop, literally the only engine it is bundled with.

Why can't this spec2 be optional until we can spin it with js and have it work in gdevelop? A tick in the settings dialog?

As I say I am fine with you continuing on a fork if you feel strongly about deprecating stuff for the sake of spec2. Make a fork of bondage that supports spec2 fully and I will join you and even contribute to it. I welcome progress always, just not when it opens new bugs.

Ask yourself what game engine can I use for yarnscript2? Is the answer only unity3d atm?

The reason json files are good for html engines has nothing to do with hurting yarn syntax. Its about the browser having a built in api to parse them. If you can only export a raw text file, that makes it much harder to use the exported data with a js module. You have to suddenly write another parser layer to get the data out.

To me anyone asking to deprecate stuff people use is being hostile. My reaction to that seems unreasonable ? 😂

You can make progress without making yarn editor less capable to export or less flexible to use. Heck, make it support both syntaxes , I am not against that.

Somebody assigned me to prevent regressions, I am doing my job damnit. Yarn editor will export json and work for both old and new spec. That is all I am advocating here.

On Thu, 21 Jan 2021, 18:54 Quetzy, notifications@github.com wrote:

Maybe I'm oversimplifying, but what if:

  • create a Yarn v2 branch, start Yarn v2 work there (no playback work)
  • create a Bondage v2 branch, yarn V2 + bondage compatibility gets done there

Nothing breaks, development doesn't get held back by anything. Worry about releases later?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/YarnSpinnerTool/YarnEditor/issues/236#issuecomment-764863025, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRRWVI44PC6BMUOIXARRRLS3BZ55ANCNFSM4WGQLNWQ .

FaultyFunctions commented 3 years ago

I think we're not on the same page. We're not advocating for completely breaking the main branch and replacing it. Yes we will do the work on a separate branch and we can submit PRs to that separate v2.0 branch as well and leave the main one undisturbed. Getting bondage.js (or some other JS runner of Yarn) up and running for 2.0 is def a priority before we can push any changes to the main branch. We're not trying to leave GDevelop in the dust. @JujuAdams even has his own runner for GameMaker that he's gonna be getting up to spec, but having a proper editor for the 2.0 spec is the top priority. The runner comes after that but still comes before we release anything.

I'm going to open up a new branch and try to get started on the 2.0 spec and see how that goes. Once I make some progress I'm going to open up a draft PR to the new branch (not the main one) and also I'll try to take a look at bondage.js as well, but I need to mess with the editor first.

I hope this makes sense. Thanks!

shdwcat commented 3 years ago

I think we need to be clear about how these dependencies are supposed to work.

The Yarn spec is (or will be) the root of all dependencies. The yarn spec moves forward independently of any engine (unity/ GDevelop/GameMaker etc).

The YarnEditor should be dependent on the spec. Of course, YarnEditor repo will use feature branches to implement new spec features before bringing them into the main/dev branch. Yarn Editor cannot have dependencies on game engine implementations keeping up with the spec. That's on the game engines.

GDevelop, if it needs to bundle YarnEditor, should be downstream of YarnEditor. I think the correct way to do this is for GDevelop to have its own fork of YarnEditor, and pull in changes from the main YarnEditor project, first into a feature branch, and then into the main/dev branch of that project once everything gets fixed up there. That way GDevelop can bundle yarn editor, never ever worrying that any work on the main YarnEditor project could cause regressions in GDevelop.

Saying that YarnEditor can't move forward because it would break GDevelop is backwards to the correct dependency relationships.

blurymind commented 3 years ago

That is all fine of course, but what if you have a bunch of issues come up on this tracker about the playtesting not supporting x or y? Do we deprecate the playtest capability completely because a specific yarn2 syntax is not supported in yarn1?

I use gdevelop just as an example - how about ct.js and other html5 game engines? Are we cutting them off too? We will end up with yarn editor splitting into two separate projects. This is why I am against the idea of doing anything that would split it into two. I want it to have both yarn1 and yarn2 spec for a while (until bondagejs catches up or we can start using yarnSpinner). That is all.

Is that not on the same page? If we have two branches that is fine too, but it will be hard for me to maintain them as a sole reviewer. I would be happy if somebody else reviews the new spec branch.

I agree the editor should be up to spec, but at the same time want the editor to be usable by everyone (spec 1 or 2) and keep it as one codebase - until spec1 becomes obsolete. Lets allow it to be flexible - is that going to be a problem that would hinder spec2? I don't seem to understand how it would. What is so different about it anyways?

If we are not removing features or syntax coloring/autocompletion, then I dont see any problems at all - so its kind of even pointless to split branches or argue about it

shdwcat commented 3 years ago

It's a bad idea to try to support yarn spec v1 and v2 from within a single release of Yarn Editor. It would be better to either have 2 release branches, one dedicated to v1 and the other to v2, or fork the v1 yarn editor to it's own project and preserve it there.

@blurymind I understand you want to preserve the existing functionality for GDevelop and other engines, and the correct way to do that is for those engines to take an explicit dependency on either v1 or v2. You really can't have it both ways without causing everyone involved in this to suffer a mountain of headaches.

If GDevelop or ct.js or any other engine wants to upgrade to v2, they need to take on that work, and it should not be tied to whether YarnEditor moves forward to v2. YarnEditor should not be forced to do backcompat work for any unknown number of game engines that might depend on it. It's up to those engines to manage their dependencies correctly, and version/fork as needed.

FaultyFunctions commented 3 years ago

We've decided to just move on with updating our own version of YarnEditor to the 2.0 spec in our own fork. This will be easier for everyone since we won't be butting heads on deciding what should and should not be updated. I'll still try to PR any bug fixes that are relevant to this repository, but I'm not making any promises. I'm closing this since this discussion wasn't really going anywhere anyway.

blurymind commented 3 years ago

Thats alright :) I would be quite interested to see how this new editor branch would deviate from the main one. I suppose that it will become the main one and this one would go on to be the legacy branch at some point.

As we talk about this, there already is another rewrite of yarn editor with react js. If I was to make a next spec editor, I would use that as a starting point, as it has more modern libraries.

Either way I am certain that bondage will catch up with the spec. Hopefully some day yarnspinner will run on anything, but for now we have to deal with this awful fragmentation of interpreters.

In any case I don't think I would honestly outright reject a pr to support the new spec, just point out any stuff that would probably create new issues on the tracker from unknowing bondagejs users. When you show users something is not supported by bondage, that tends to drive some to remedy the problem with prs.