YarnSpinnerTool / YarnSpinner

Yarn Spinner is a tool for building interactive dialogue in games!
https://yarnspinner.dev
MIT License
2.3k stars 201 forks source link

Deprecate using `ConvertTo` for `Value` #357

Closed sanbox-irl closed 7 months ago

sanbox-irl commented 1 year ago

I am working on a non-C# based implementation of the Yarn Interpreter.

At various times, internally, casts are done between a Value of String, f32, or Bool into any of the other three. Internally, the C# interpreter uses System.Convert to do those conversions.

This creates a semver hazard for other interpreters and takes the standard out of the control of YarnSpinner. It also forces another implementation to handle some fairly odd design decisions of C# ("true" => true, etc).

I would propose instead that this project handles those conversions manually rather than calling System.Convert. Since the type id of the caller is available, and of the target type, it should boil down to just a few switch statements

McJones commented 7 months ago

After discussion we have decided to close this issue and keep with the existing implementation of using System.Convert. While it is true this means we follow the pattern of converting the same way C# does if we were to change there only leaves two options to handling value conversion:

The first simply swaps C# design decisions for a different language, the same arguments apply. The second approach means we now can't even go "oh we convert values the same way as X" and will have to build up conversion tables and point people to that when they want to know how values are converted. And we'd likely end up just recreating an existing languages approach because there are only a few types, leading us back to the first option, picking another languages approach.

So that realistically means we either choose to follow C# or pick another language as the guide. So what language should be chosen as the guide? As C# is the language Yarn Spinner is implemented in we don't see any advantage in changing away from System.Convert. There is no improvement for the users as they will still have to learn a conversion ruleset regardless.

This takes us back to the start, we are closing this issue as we don't see any significant reason or advantage to change. Thanks for the proposal.