Help optimizing my skript, for mount.

[Zach R]

Hey everyone, I have a "working" piece of code here,
however I am very new to all this and am confident someone could do this significantly better. Occasionally players get teleported into the void, the horse doesn't get de spawned...

Essentially, right clicking this item puts you on a mount, rightclicking again, or dismount removes the horse (via void tp to avoid in humane death animation). There are a few things in there such as teleport players that helps dismount the player from the horse in order prevent being teleported in the void. I believe it is SkBee responsible for the NBT, I use scoreboard tags so I can check them in game if necessary and so on so on. Oh yes, only horses spawned by the item should be removed on dismount or item toggle! Ideally regular horses would act normally!

Anyone who has any ideas how to optimize, simplify, make this work more consistently, (without vanilla commands would be nice too, but I'll take any help I can get) I'd love to see what you come up with!!

on rightclick holding a emerald:
    if name of event-item is "&cHorse Whistle":
        if {whistlereset.%player%} is not set:
            if player is not riding a horse:
                add "Mount_Horse" to the scoreboard tags of event-entity
                set {whistlereset.%player%} to true
                play sound "entity.parrot.ambient" to player
                wait 1 second
                play sound "entity.horse.gallop" to player
                wait 0.1 second
                play sound "entity.horse.ambient" to player
                send "&aHorse Summoned"
                spawn horse at player
                add "{Tame:1b,Variant:259,SaddleItem:{id:saddle,Count:1},Age:0}" to last spawned entity's nbt
                make player ride spawned horse
            else if player is riding a horse:
                make player run command "execute as @a[distance=0..2] at @s run tp @s ~ ~ ~" as op
            wait 5 seconds
            clear {whistlereset.%player%}
        else if {whistlereset.%player%} is set:
            message "&c&oThe whistle is still wet with saliva, try again later."

on dismount:
    if player has scoreboard tag "Mount_Horse":
        send "&cHorse Unsummoned"
        make player run command "execute as @a[distance=0..2] at @s run tp @s ~ ~ ~" as op
        wait 1 tick
        make player run command "tp @e[type=horse,sort=nearest,limit=1] ~ ~-300 ~" as op 
        wait 10 tick
        remove "Mount_Horse" from the scoreboard tags of event-entity

 

[AFRICA1155]

• make player run command "execute as @a[distance=0..2] at @s run tp @s ~ ~ ~" as op

Maybe instead you could do something like:

make player dismount their vehicle

I personally don't like seeing commands in the console of unimportant stuff executed by skript, it just helps overall keeping things a bit cleaner.
Aside from that, I don't know how you could make it much more efficient. It's a lot cleaner than most things I've written, lol.

What you could always do though, is to create a function out of your code, and just have the function be called whenever you just do something such as rightclicking with the emerald. This way you wouldn't have to copy the whole code if you wanted to have the code be ran in a different instance, all you'd have to do is to just call the function again and this helps keep things cleaner in the long run, it would also make it way easier to integrate your code into other projects.
[doublepost=1647171422,1647170569][/doublepost]Btw this would be an example of how to dismount the horse and get in the void without commands, i'm not that much of a scoreboard guy, so unfortunately can't help much with that. Btw, you can almost always just have variables save and track information on players because almost everything can be done in skript anyways. Having to rely on scoreboards also isn't always the best because when you delete your world or do a world reset, you'd lose all data, it depends on what you want ofc.

set {_loc} to location of player's vehicle

set y-coordinate of {_loc} to -50 #you can adjust this value so the horse ends up in the void
set {_horse} to player's vehicle
make player dismount their vehicle
teleport {_horse} to  {_loc}

[doublepost=1647174142][/doublepost]

• add "{Tame:1b,Variant:259,SaddleItem:{id:saddle,Count:1},Age:0}" to last spawned entity's nbt

And you also just add a custom nbt tag to to the horse to check if it was spawned with a whistle, here's your nbt with an additional custom tag

{Tame:1b,Variant:259,SaddleItem:{id:saddle,Count:1},Age:0,Tags:["SpawnedWithWhistle"]}

then check if it contains the tag

if {_horse}'s nbt contains "SpawnedWithWhistle":
    #TELEPORT HORSE OR DO WHAT YOU WANT

 

[Zach R]

wow, you are brilliant my friend! Ton's of great ideas, I intend to implement all of them! Exactly what I needed, sometimes doing something on your own you get set in a away and don't think of the other options. It is very must appreciated!
[doublepost=1647198285,1647194686][/doublepost]Hey, so I am soooooo close to having everything working, but I've spent a little bit too long trying to get the horse teleported to the void. And can not figure out for the life of me why it wont happen! I've tried what you said, nothing, even tried straight up coords, nothing.

on dismount:
    set {_horse} to player's vehicle
    if {_horse}'s nbt contains "SpawnedWithWhistle":
        send "&cHorse Unsummoned"
        set {_loc} to location(-148, 149, -122, world "SurvivalUltimate")
        teleport {_horse} to {_loc}

teleport {_horse} to location(-148, 149, -122, world "SurvivalUltimate")

This location is loaded in, the made the horse "glow" to test it was the right entity assigned to the tag. Everything is good except I can't get it to teleport!

Here's the full code:

Spoiler: Full Code

on rightclick holding a emerald:
    if name of event-item is "&cHorse Whistle":
        if {whistlereset.%player%} is not set:
            if player is not riding a horse:
                set {whistlereset.%player%} to true
                play sound "entity.parrot.ambient" to player
                wait 1 second
                play sound "entity.horse.gallop" to player
                wait 0.1 second
                play sound "entity.horse.ambient" to player
            
                send "&aHorse Summoned"
                spawn horse at player
                add "{Tame:1b,Variant:259,SaddleItem:{id:saddle,Count:1},Age:0,Tags:[""SpawnedWithWhistle""]}" to last spawned entity's nbt
                make player ride spawned horse
            
            else if player is riding a horse:
                set {_horse} to player's vehicle
                if {_horse}'s nbt contains "SpawnedWithWhistle":
                    make player dismount their vehicle
                else:
                    send "&cThis horse was not summoned."
                
                
            wait 5 seconds
            clear {whistlereset.%player%}
            
            
        else if {whistlereset.%player%} is set:
            message "&c&oThe whistle is still wet with saliva, try again later."

on dismount:
    set {_horse} to player's vehicle
    if {_horse}'s nbt contains "SpawnedWithWhistle":
        send "&cHorse Unsummoned"
        set {_loc} to location(-148, 149, -122, world "SurvivalUltimate")
        teleport {_horse} to {_loc}
        kill {_horse}

 

[AFRICA1155]

• set {_loc} to location(-148, 149, -122, world "SurvivalUltimate")

No problem, have you taken a look at this location? There's a great chance that the y-coordinate '149' is likely not in the void. But at y:149
Coordinates are listed in the order: X, Y, Z. So there might be a chance that this was an issue.

Another problem I've had myself, is that setting a location in a different world, doesn't work when you do it using:

• set {_loc} to location(-148, 149, -122, world "SurvivalUltimate")

But it does using the following syntax:

        set {_loc} to location (-148, -122, 149)
        set {_loc}'s world to world "SurvivalUltimate"
        teleport {_horse} to {_loc}

I've also had this problem myself using skript 2.6, so no worries man! I hope this fixes it.
Btw,

I'm glad to be able to help. I've learnt most of this stuff the hard way, by figuring out that my code wasn't efficient enough and trying to improve it just like you did, so I'd rather have no one else spent long on this kind of optimizing stuff. Custom tags is definitely the underrated way to go with mobs to avoid having thousands of variables attached to unloaded mobs, maybe of even deleted worlds. All of those variables take up lots of space and slow everything down just to have them loaded, let alone looped once. You can't acces or do anything with unloaded mob anyways, so there's no point in attaching variables to them, unless you have a special reason for them.

 

[Zach R]

Hey thanks again for the great advice! Yeah definitely find a few issues every once and awhile.
So I've added you snippit of code in and the issue prevails. (I do not actually need to assign the world, just happened to have that in what I showed. I can confirm the location is set, and the correct horse is set to variable, however it still will not be teleported!! I have no idea why?!
I additionally can't teleport the event-entity (player dismounting) to {_loc}... I confirmed all the variables are correct with this.

broadcast "%event-entity%, %{_horse}%, %{_loc}%"

Lol, I'm super confused any more ideas?
[doublepost=1647227750,1647227246][/doublepost]Omg I'm feeling pretty dumb. I've solved it.
I just added wait 1 tick before teleporting of entity. That's all it required.
Thank you so much for your help! It's working awesomely now! So consistent, so good!!
Seriously thank you, love this community so amazingly helpful.
Hey btw why does everyone put an underscore before a variable {_horse} instead of just {horse}?

 

[AFRICA1155]

Hey thanks again for the great advice! Yeah definitely find a few issues every once and awhile.
So I've added you snippit of code in and the issue prevails. (I do not actually need to assign the world, just happened to have that in what I showed. I can confirm the location is set, and the correct horse is set to variable, however it still will not be teleported!! I have no idea why?!
I additionally can't teleport the event-entity (player dismounting) to {_loc}... I confirmed all the variables are correct with this.

broadcast "%event-entity%, %{_horse}%, %{_loc}%"

Lol, I'm super confused any more ideas?
[doublepost=1647227750,1647227246][/doublepost]Omg I'm feeling pretty dumb. I've solved it.
I just added wait 1 tick before teleporting of entity. That's all it required.
Thank you so much for your help! It's working awesomely now! So consistent, so good!!
Seriously thank you, love this community so amazingly helpful.
Hey btw why does everyone put an underscore before a variable {_horse} instead of just {horse}?

Hey, no problem, I'm glad you got the problem sorted out! If you need any further help you can always ask me at anytime.

But to answer your question about the underscore let me briefly try to explain local variables, since they're a very important kind of variable to understand and can make things much easier.

Adding an underscore before a variable makes it a local variable.

If I'd have to explain what local variables are I'd describe it as variables that only exist and must be set during one instance (=event or whatever it is). Outside of that instance the variable doesn't exist, hence why it's local. You often use local variables to save and use information during one specific event, but don't want it to be used across the rest of the server and don't want it to exist/save after the event has passed. That variable with it's set value only exists within that event. Multiple local variables with the same name can exist at the same time and won't conflict with other local variables on the server with the same name.

In order to tell Skript you want this variable to be a local variable all you need to do is prefix the variable name with an _
for example: '{horse}' becomes --> '{_horse}'
However since the variable only exists within the event, you can't refer or call for a local variable outside of the event, which makes sense because the local variable doesn't exists outside of that one instance. This is also means the variable won't be saved and that it can save you a ton of space if possible, this also means that you won't have to delete this variable, because it doesn't exist anymore after the event has passed.

If you need the variable outside of that one instance then just use normal variables, but in this case it's better to use a local variable, because we don't need the horse or variable anymore after the event has passed, nor outside of it. Btw, directly setting an entity as a variable can often not work anymore after the server has restarted. You can however save the entity's uuid as variable and check if the UUID of the entity and the variable is the same to confirm if it's the right entity, even after the server has restarted. So in this case it's completely fine to use a local variable just to refer back to the same horse later in the event.
To read more about the types of different variables, you can always visit this page: https://dev.bukkit.org/projects/skript/pages/variables-explanation

Anyways, I'm glad to be able to help out, and it's okay to make mistakes, most of the mistakes we make on here, we didn't even consider to be a possible error in our code, until we try to things differently, we all often get stuck like that, here


Anyways, have a good day my friend, if you need any further help, feel free to DM me or to ask other comrades on this forum. I'm new here too btw and very happy that I could help you out, buddy

 

[Zach R]

wow! That was extremely helpful once again,
I was starting to have a feeling it was used to identify the variable in some way but you explained it way better then I could ever imagine! I really appreciate all the effort you put into helping me out! I definitely have a few more challenges working out so maybe you will hear from me again!
Once again, appreciated! Thank you!

 

[timeis]

I would try to avoid making skript run commands unless you 100% have to