Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

starts with condition bug #3312

Closed
ghost opened this issue Aug 16, 2020 · 4 comments
Closed

starts with condition bug #3312

ghost opened this issue Aug 16, 2020 · 4 comments
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update. priority: low Issues that are not harmful to the experience but are related to useful changes or additions.

Comments

@ghost
Copy link

ghost commented Aug 16, 2020

Description

the more ors you have in starts with condition, the smaller chance for the condition to return true, even if the condition is definitely met

Steps to Reproduce

command /teststarts:
	trigger:
		if "1" starts with "1" or "2" or "3" or "4" or "5" or "6" or "7" or "8" or "9":
			send "true"
		else:
			send "false"

Expected Behavior

expected to always be true

Errors / Screenshots

None

Server Information

  • Server version/platform: 1.12.2 Paper
  • Skript version: 2.5beta2

Additional Context

@ShaneBeee
Copy link
Contributor

According to the docs, this doesn't even support or

as You can see, it says "text" not "texts"
Screen Shot 2020-08-16 at 4 05 01 AM

@jaylawl
Copy link
Contributor

jaylawl commented Aug 16, 2020

command /teststarts:
	trigger:
                if ("1", "2", "3", "4", "5", "6", "7", "8", "9") contains "1":
			send "true"
		else:
			send "false"

@Moderocky
Copy link
Member

Moderocky commented Aug 16, 2020

What’s happening is - because the contains check only accepts a single text - it is first resolving the (A or B or C) part, so there is only a 1/n chance that it is the matching “1” - perhaps it would be better to put this as a feature request?

Something like if %thing% contains (all|any) of %strings%

(This might need to be adapted, since contains can be for list contains, string contains, etc.)

@FranKusmiruk
Copy link
Member

What’s happening is - because the contains check only accepts a single text - it is first resolving the (A or B or C) part, so there is only a 1/n chance that it is the matching “1” - perhaps it would be better to put this as a feature request?

Something like if %thing% contains (all|any) of %strings%

(This might need to be adapted, since contains can be for list contains, string contains, etc.)

The contains condition already supports this for strings, there's the (any|all|none) of %texts% contains %texts% pattern. Anyways, marking this one as a bug since the starts with condition should ideally support or lists properly or in general, we may as well add a warning when a user tries to use an or list on a condition that doesn't support them such as this case.

@FranKusmiruk FranKusmiruk 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 Aug 16, 2020
@APickledWalrus APickledWalrus added the completed The issue has been fully resolved and the change will be in the next Skript update. label Oct 5, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update. priority: low Issues that are not harmful to the experience but are related to useful changes or additions.
Projects
None yet
Development

No branches or pull requests

5 participants