Looking for improvement Suggestions - My Skript-knowledge is pretty old and rusty..

[pcrdex]

So I've made a Report Skript with using AdvancedBans (configurable obv) and I'm looking for improvements:

command /report [<player>] [<text>]:
    trigger:
        if arg-1 is set:
            if arg-1 is a player:
                if arg-2 is set:
                    send "&e[&6&lREPORT&e] You reported &f%arg-1% for &e%arg-2%&f!"
                    add arg-1 to {reports::*}
                    add 1 to {reportamount::%arg-1's uuid%}
                    add arg-2 to {reports::%arg-1's uuid%::*}
                    loop all players:
                        if player has permission "reports.permission":
                            send "&e[&6&lREPORT&e] &fNew Report: &eName: &f%arg-1% &8- &eReason: &f%arg-2% &8- &eUser: &f%player%&f!"
                            stop
                else:
                    send "&e[&6&lREPORT&e] &fPlease choose a valid reason!"
            else:
                send "&e[&6&lREPORT&e] &fPlease choose a valid Player!"
        else:
            send "&e[&6&lREPORT&e] &fPlease use: /report [player] [reason]"

command /reports [<offline player>] [<text>]:
    permission: reports.permission
    trigger:
        if {reports::*} contains arg-1:
            set {player} to arg-1
            if {reports::%arg-1's uuid%::*} contains arg-2:
                set {reason} to arg-2
                set {_r} to a new chest inventory with 5 rows named "&e&l%arg-1%'s REPORTS"
                set slot (integers between 0 and 53) of {_r} to gray stained glass pane named " "
                set slot 13 of {_r} to skull of arg-1 named "&e&l%arg-1%" with lore "%nl%&7Reports: &6%{reportamount::%arg-1's uuid%}%%nl%&7Reasons: &6%{reports::%arg-1's uuid%::*}%"
                set slot 29 of {_r} to lime dye named "&a&lDELETE ALL REPORTS" with lore "%nl%&7|| &fClick here to clear %arg-1%!"
                set slot 31 of {_r} to red dye named "&4&lBAN PLAYER" with lore "%nl%&7|| &fClick here to ban %arg-1%!"
                set slot 33 of {_r} to orange dye named "&6&lWARN PLAYER" with lore "%nl%&7|| &fClick here to warn %arg-1%!"
                open {_r} to player
            else:
                send "&e[&6&lREPORT&e] &fThere are no open reports with that reason for %arg-1%!"
        else:
            send "&e[&6&lREPORT&e] &fThere are no open reports for %arg-1%!"

on inventory click:
    if name of player's current inventory contains "REPORTS":
        cancel event
        if event-slot is 29:
            send "&e[&6&lREPORT&e] &fYou cleared all Reports for %{player}%!"
            delete {player}
            delete {reason}
            delete {reports::*}
            delete {reports::%{player}'s uuid%::*}
            close player's inventory
        if event-slot is 31:
            set {_p} to a new chest inventory with 4 rows named "&e&lCHOOSE DURATION"
            set slot (integers between 0 and 53) of {_p} to gray stained glass pane named " "
            set slot 11 of {_p} to paper named "&c1 Hour"
            set slot 12 of {_p} to paper named "&c4 Hours"
            set slot 13 of {_p} to paper named "&c12 Hours"
            set slot 14 of {_p} to paper named "&c24 Hours"
            set slot 15 of {_p} to paper named "&c7 Days"
            set slot 21 of {_p} to paper named "&c30 Days"
            set slot 23 of {_p} to paper named "&4PERMANENT"
            open {_p} to player
        if event-slot is 33:
            make player execute command "/warn %{player}% %{reason}%"
            send "&e[&6&lREPORT&e] &fYou successfully warned %{player}% for %{reason}%!"
            delete {player}
            delete {reason}
            delete {reports::*}
            delete {reports::%{player}'s uuid%::*}
            close player's inventory
    if name of player's current inventory contains "CHOOSE DURATION":
        if event-slot is 11:
            make player execute command "/tempban %{player}% 1h %{reason}%"
            send "&e[&6&lREPORT&e] &fYou successfully banned %{player}% for 1 Hour!"
        if event-slot is 12:
            make player execute command "/tempban %{player}% 4h %{reason}%"
            send "&e[&6&lREPORT&e] &fYou successfully banned %{player}% for 4 Hours!"
        if event-slot is 13:
            make player execute command "/tempban %{player}% 12h %{reason}%"
            send "&e[&6&lREPORT&e] &fYou successfully banned %{player}% for 12 Hours!"
        if event-slot is 14:
            make player execute command "/tempban %{player}% 24h %{reason}%"
            send "&e[&6&lREPORT&e] &fYou successfully banned %{player}% for 24 Hours!"
        if event-slot is 15:
            make player execute command "/tempban %{player}% 7d %{reason}%"
            send "&e[&6&lREPORT&e] &fYou successfully banned %{player}% for 7 Days!"
        if event-slot is 21:
            make player execute command "/tempban %{player}% 30d %{reason}%"
            send "&e[&6&lREPORT&e] &fYou successfully banned %{player}% for 30 Days!"
        if event-slot is 23:
            make player execute command "/ban %{player}% %{reason}%"
            send "&e[&6&lREPORT&e] &fYou successfully banned %{player}% permanent!"
        if event-slot is 11, 12, 13, 14, 15, 21 or 23:
            delete {player}
            delete {reason}
            delete {reports::*}
            delete {reports::%{player}'s uuid%::*}
            close player's inventory

 

[Doodle]

Before I continue, thank you for putting this in a code block.

1. on line 69, you forgot to cancel the event for the inventory.

2. for the {player} and the {reason} variables, use a list variables containing the player's uuid. For example:

command /reports [<offline player>] [<text>]:
    permission: reports.permission
    trigger:
        if {reports::*} contains arg-1:
            set {player::%player's uuid%} to arg-1
            if {reports::%arg-1's uuid%::*} contains arg-2:
                set {reason::%player's uuid%} to arg-2

this way it will save across the player, and not universally. It's hard to explain, but if two players used the /reports command at the same time, the variable would only be set to one player, which would be very confusing.

3. Skript has custom ban effects. you don't have to make the player execute the command, as the command might not exist in the server or the player might not have the permissions.

4. the way you made this gui... it's really easy to hack into it... you can just rename a chest to "REPORTS", open it, and now you have access to the menu...
to fix this, I'd recommend either using metadata, which is a pain in the ass, or just using skript-gui.

there's probably more stuff that I missed, but I do not feel like fully examining the code. (it is currently almost midnight for me as I am making this comment.)

Regardless, good job! Keep it up!