SkriptLang / Skript

Skript is a Spigot plugin which allows server admins to customize their server easily, but without the hassle of programming a plugin or asking/paying someone to program a plugin for them.
https://docs.skriptlang.org
GNU General Public License v3.0
1.06k stars 367 forks source link

[horrible high priority bug] If player has played before: doesnt work if player has joined server for first time. needs to log out first #1918

Closed TsCode1 closed 5 years ago

TsCode1 commented 5 years ago

Description

If you use "If player has played before:" it works fine unless the player has joined for the first time, it will not recognize this player as played before. The players need to log out at least once. A fix for this is checking if player is online first, would be a double check though.

Steps to Reproduce

command /check [offlineplayer>: if player has played before: work else doesnt work on first join

Expected Behavior

I expect it to work on first join because if a player has joined before it means he played before.

Errors / Screenshots

Server Information

JoyousJohn commented 5 years ago

Try with Skript 2.3.5?

ShaneBeee commented 5 years ago

I could be wrong, but I believe this has nothing to do with Skript, this is an issue with the method from the BukkitAPI

Pikachu920 commented 5 years ago

but working around it is as easy as return p.isOnline() || p.hasPlayedBefore(). It's just a question of: should we?

Nicofisi commented 5 years ago

I might misunderstand something completely but isn't the current behavior the whole point of the condition? To check whether this is not the first time the player is on the server?

ShaneBeee commented 5 years ago

This doesn't really need to be re-written since you could easily just do

command /check <offline player>:
    trigger:
        if arg-1 is online:
            send "Has played"
        else if arg-1 has played before:
            send "Has played"
        else:
            send "Has NOT played"
TsCode1 commented 5 years ago

In my opinion the expected behavior for has played before includes an online player and thus you should add pikachu's double check if there is no way around it. Otherwise the only way around for me personally is doing the whole script twice, checking first if player has played before, then else if online with the duplicated script. You can't use "or" in my case when I mess around with completed names

Nicofisi commented 5 years ago

Oh, I see what you mean now, but that's not the point of the condition, it was originally intended to work as on first join:, like

on join:
    if player hasn't played before:
        send "Hey, welcome to our server for the first time!"
        give 5 potatoes to the player

that's why I couldn't get why you wanted it changed at all at first. Pretty sure there's even an example like that in the docs, so I'm against changing that honestly.

You could just make a function and use it everywhere :eyes:

TsCode1 commented 5 years ago

Oh, I see what you mean now, but that's not the point of the condition, it was originally intended to work as on first join:, like

on join:
    if player hasn't played before:
        send "Hey, welcome to our server for the first time!"
        give 5 potatoes to the player

that's why I couldn't get why you wanted it changed at all at first. Pretty sure there's even an example like that in the docs, so I'm against changing that honestly.

You could just make a function and use it everywhere 👀

but there is on first join why would u ever use on join, if player hasnt played before.

Also if this is a spigot problem, I will happily report it to the papermc devs. I am assuming it's because bukkit/spigot only saves this info on a log out.

ShaneBeee commented 5 years ago

I wouldn't really call this a "spigot problem" since its not technically an issue.

The only time someone would really be using the hasPlayedBefore method, wouldn't be in a join event. So when a player is joining, if they haven't played before, do something. The only other time you would use it is checking if an offline player has played before.

If the player is currently online, there would be absolutely no need to check if they have played before.

That being said, its not a BUG with Spigot, since you wouldn't check an ONLINE player to see if they have played before, that just makes no sense.

As for using this in Skript, you are talking about using it in a command, checking if the player argument in the command has played before. Simple solution:

command /check <offline player>:
    trigger:
        if arg-1 is online:
            send "Has played"
        else if arg-1 has played before:
            send "Has played"
        else:
            send "Has NOT played"

I really see no need to re-write this condition in script, since it perfectly matches how the BukkitAPI handles it, and there is absolutely nothing wrong with it. The BukkitAPI is full of all sorts of things like this, they aren't "broken" per se, they just need to be handled properly in your code.

Also a little note to add, the method is called hasPlayedBefore() ... PLAYED being the key word here. If the player is ONLINE, they are currently PLAYING the game, and PLAYED (being past tense) doesn't fit the bill.

TsCode1 commented 5 years ago

If that's the case it should be called hasLoggedOutBefore, because an online player did play in the past no matter how much you try to twist the words.

ShaneBeee commented 5 years ago

If that's what you want to think sure.... but Spigot isn't going to change it for that reason.

If the player is online, they haven't PLAYED before, because PLAYED would indicate past tense, indicating the player has a past played session. Their current session is PLAYING, therefor their last session is non existent.

Its been hasPlayerBefore() since the dawn of Bukkit time (or whenever they added it) and no ones made a deal with it, because the devs know how to properly use it.

Nicofisi commented 5 years ago

Don't want to be the one closing it, but just to note, I fully agree with @ShaneBeee

TsCode1 commented 5 years ago

alright close the issue

TheBentoBox commented 5 years ago

I see both sides but I don't think we should modify the behavior of syntaxes which are meant to essentially be direct mappings of the Bukkit API.