Add deprecation message for $ sign on script loading
gerzytet opened this issue · 11 comments
Code such as:
#MESSAGE "you need $3 to buy a sponge"
will work in 2.1.8, but not 2.2, thanks to the new interpolation features added by c11f67e.
To avoid breaking such code, placeholder interpolation needs to be explicitly specified by requiring use of the , or by declaring the string in some special way (such as python's raw strings, or "f-strings")
I believe the former is easier to do.
I think we can address this issue with the config file
Maybe add an option like useAutomaticPlaceholderConversion=true
so that the new comers (usually who don't understand String concatenation) will benefit from the automatic conversion, yet the previous code flow can be compatible if set the option to false
This will require #167 to be done first though.
I still believe that changing the syntax is the best way to go. The current syntax defines an entirely new special-meaning string character other than \ (unusual in most languages).
If it was a global config, it would introduce further inconsistency to the language. We would have to start telling people to change their configuration options just to be able to use the new feature, but changing it may introduce the incompatibility issues with other triggers.
If it was local to triggers, we would have to worry about adding that config option to every single trigger, including named triggers, who don't have one.
If the goal is to release the feature with backwards compatibility, consistency, in a way that is simple to implement and understand, changing the syntax is the only way to go.
If you don't like the look of "\$playername is in \$worldname"
consider this: $"$playername is in $worldname"
Wouldn't that disqualify the justification of this commit entirely?
The reason for this commit is that often, new comers put placeholders inside the string and presumably think it will work, just like how VT works.
Sadly, it's indistinguishable whether the $ sign is a placeholder or not.
For example, $3 can be a placeholder named 3, but it's impossible to know the differences.
Though c11f67e also introduces the escape sequence, so one who wants to fix the problem can add escape sequence instead.
#MESSAGE "you need \$3 to buy a sponge"
That fixes the problem for anyone writing new code, but still requires server owners track down and apply those fixes when they switch to the new version.
In other words, it's incompatible.
If you want to implement the syntax the way it currently is, you need to be very careful that everyone knows what’s coming, and that you give people time to adjust, meaning that the feature probably isn’t right for 2.2
That would begin by making an unescaped $ in strings a warning or an error.
It’s just not fair to suddenly implement a feature that could break existing code without telling everyone
Will add deprecation message to the 2.2 release version.
Automatic placeholder conversion will be added to 3.0.0
sounds good to me
I think the best time to display these messages would be on plugin load/reload