Replace vanishedPlayers list with set

[mdcfe]

Not sure if there is any particular reason to keep it ordered, but for now I've used a LinkedHashSet.

Closes #1789 by implementing the alternative solution.

Test builds

Commit	Link
0237c71	EssentialsX-2.0.1-pr1796.jar

[drtshock]

Hm. I agree with the change, I just don't like that we have to name a method like that in order to preserve compatibility.

[mdcfe]

Yeah, I didn't massively like that method name either.

We could just change the return type, but that would be quite annoying for a lot of people.

[drtshock]

Yeah that would break plugins currently hooking it.

[caojohnny]

This may be replaced by:

return new ArrayList<>(this.vanishedPlayers);

I'd also like to question the validity of returning a modifiable collection. While changes to this collection might not cause changes, returning a modifiable collection in getVanishedPlayersSet() would, which may confuse some people. I recommend perhaps using Collections#unmodifiableXYZ(...) to wrap the result instead.

[SupaHam]

I agree with this logic. Please do use unmodifiableList().

[SupaHam]

@md678685 if the return type is not going to change, we need to track this so that when we do decide to push a whole bunch of breaking changes we will be able to clean up the codebase with it. A collection should be returned, not a List or a Set.

Edit: but of course, if you do keep the extra method, then it'll just be messier.

[caojohnny]

While not a fully developed idea, I think the priority between compatibility and functionality should be sorted out right now. In the case that compatibility is favored, in the worst case, you'd probably actually want to keep the vanishedPlayers field a List type and check for duplicates every time, at a cost to performance.