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

Default/placeholder values in a required command argument makes all subsequent arguments optional. #5238

Open
1 task done
sovdeeth opened this issue Dec 8, 2022 · 3 comments
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something. priority: lowest "Nice to have" updates that are not required (tiny low impact bug fixes or QoL enhancements).

Comments

@sovdeeth
Copy link
Member

sovdeeth commented Dec 8, 2022

Skript/Server Version

[23:08:57 INFO]: [Skript] Skript's aliases can be found here: https://github.com/SkriptLang/skript-aliases
[23:08:57 INFO]: [Skript] Skript's documentation can be found here: https://docs.skriptlang.org/
[23:08:57 INFO]: [Skript] Skript's tutorials can be found here: https://docs.skriptlang.org/tutorials
[23:08:57 INFO]: [Skript] Server Version: git-Paper-125 (MC: 1.19.2)
[23:08:57 INFO]: [Skript] Skript Version: 2.6.4
[23:08:57 INFO]: [Skript] Installed Skript Addons: None
[23:08:57 INFO]: [Skript] Installed dependencies:
[23:08:57 INFO]: [Skript]  - WorldGuard v7.0.8-SNAPSHOT+2200-4a21bf4

Bug Description

When giving required type arguments default values in commands (<number=1>), Skript automatically makes that argument and all subsequent arguments optional. See the provided snippet in Steps to Reproduce.

Expected Behavior

This should, at most, make just the argument with the default value optional.

Steps to Reproduce

command test <number> <number=1> <number>:
    trigger:
        send "%arg-1%, %arg-2%, %arg-3%"

Errors or Screenshots

image

Other

Personally, I'm of the opinion it shouldn't make anything optional, and optional arguments should only be those surrounded by []. The behavior of the argument becoming optional is un-intuitive, as normally the only optional arguments in commands are surrounded by []. Since this argument isn't, it appears required but actually isn't. Enforcing [] for all optional commands would also mean commands would more closely follow the generally accepted convention for Minecraft commands:
image

Agreement

  • I have read the guidelines above and affirm I am following them with this report.
@Fusezion
Copy link
Contributor

Fusezion commented Dec 8, 2022

As for your "other" portion I'm creating a discussion in proposals rn on that topic and how we might be able to go about changing it. just will cause a fairly heavy breaking change

@AyhamAl-Ali AyhamAl-Ali added bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. priority: low Issues that are not harmful to the experience but are related to useful changes or additions. labels Dec 8, 2022
@TheLimeGlass
Copy link
Collaborator

TheLimeGlass commented Jan 16, 2023

When you define the default argument, that's insinuating that the argument is opinional when provided, and because of that every other argument after must be optional when the default argument is provided. Skript does this because if an argument is a string, everything is going to go wrong.

This could throw an error when all the arguments are not strings, but this is intended behavior.

Skript could be able to tell the amount of arguments to improve this suggestion.

@TheLimeGlass TheLimeGlass added enhancement Feature request, an issue about something that could be improved, or a PR improving something. priority: lowest "Nice to have" updates that are not required (tiny low impact bug fixes or QoL enhancements). and removed bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. priority: low Issues that are not harmful to the experience but are related to useful changes or additions. labels Jan 16, 2023
@sovdeeth
Copy link
Member Author

sovdeeth commented Jan 16, 2023

Skript does this because if an argument is a string, everything is going to go wrong.

That makes sense. However, I think this behavior is hidden and not intuitive, and that we should error (or warn) if a required argument has a default value. Since every variable with a default value is optional, we should ensure that users explicitly make an argument optional before giving a default value in order to avoid confusion about why unrelated arguments are becoming optional.
Basically just what I said in the Other section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something. priority: lowest "Nice to have" updates that are not required (tiny low impact bug fixes or QoL enhancements).
Projects
None yet
Development

No branches or pull requests

4 participants