Bug: contains not working propperly with lists stored in options

[JustDylan23]

options:
	worlds: "world", "world_nether", "world_the_end" and "spawn"
command /test:
	trigger:
		if {@worlds} contains "%player's world%":
			broadcast "1"

for some reason the contains statement is not working correctly with options,
I debugged it and the following thing seemed to work so this is probbably an issue with expressionlist comparisons.

options:
	worlds: "world", "world_nether", "world_the_end" and "spawn"
command /test:
	trigger:
		set {_w::*} to {@worlds}
		if {_w::*} contains "%player's world%":
			broadcast "1"```

[TheLimeGlass]

Addition: could only reproduce when more than three values are defined, valid issue.

Note: The player's world is not related to the issue, it's in general. The contains condition has been modified a lot by Bensku over the life of this fork, potential issue in the condition.

[28andrew]

I was able to reproduce the issue with this code:

options:
    worlds: "world", "world_nether", "world_the_end" and "spawn"
command /test3:
    trigger:
        if {@worlds} contains "%player's world%":
            broadcast "1"

but not this code

options:
    list: "apple", "banana", "oreo", and "chicken"

command /test <text>:
    trigger:
        set {_list::*} to "apple", "banana", "oreo", and "chicken"
        if {_list::*} contains arg-1:
            send "%arg-1%: Yes"
        else:
            send "%arg-1%: No"

command /test2 <text>:
    trigger:
        set {_list::*} to {@list}
        if {_list::*} contains arg-1:
            send "%arg-1%: Yes"
        else:
            send "%arg-1%: No"

I'll keep looking into this issue and maybe PR if I can find a fix.

[28andrew]

I have identified why the issue happens. Keep in mind implementations of Expression#check should (and I think all implementations in Skript follow this) return true if getAnd() is true and all elements pass the checker, or if getAnd() is false and any of the elements pass the checker.

When you check if a list variable contains a string, it will be true if any of the contents of that list variable contain the item (keep in mind getAnd() of a variable is always true):
https://github.com/SkriptLang/Skript/blob/master/src/main/java/ch/njol/skript/conditions/CondContains.java#L104-L115

This is not an issue specific to being used with options. This is if an ExpressionList is passed to CondCompare.

However when you use an ExpressionList (if "a", "b", and "c" contains "c":; where "a", "b", and "c"), the checks are done individually and fail if any contents of the ExpressionList do not contain the item if getAnd() is true (getAnd() is true if you use and when making the ExpressionList) (Special case with strings: if the ExpressionList has strings, it will also check if the string is inside that string):
https://github.com/SkriptLang/Skript/blob/master/src/main/java/ch/njol/skript/conditions/CondContains.java#L126-L138

Now you may ask, what happens if you use or (getAnd() = false) in an ExpressionList. Well that does make getAnd() false but or will make the containers in CondContains only have a random element from ExpressionList. So that's not intended behavior either.

I hope I explained it clearly enough.

Proposed Solution: Don't use Expression#check (as it considers getAnd() into the result) and instead loop through the container (the container is the "haystack" when checking if something contains something): https://github.com/SkriptLang/Skript/blob/master/src/main/java/ch/njol/skript/conditions/CondContains.java#L101

TL;DR: When variables are used in the contains condition they get special treatment and getAnd() is ignored. This isn't the same case for ExpressionList though.

[28andrew]

I'm not going to PR because I'm not sure of the best way to fix it and if that will cause any unforeseen side effects with other use cases of the contains condition.