Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return of the Block (a discussion) #3315

Closed
ShaneBeee opened this issue Aug 16, 2020 · 5 comments
Closed

Return of the Block (a discussion) #3315

ShaneBeee opened this issue Aug 16, 2020 · 5 comments
Labels
completed The issue has been fully resolved and the change will be in the next Skript update. enhancement Feature request, an issue about something that could be improved, or a PR improving something.

Comments

@ShaneBeee
Copy link
Contributor

ShaneBeee commented Aug 16, 2020

Description

I wanted to open up a discussion regarding the block object.

A common misconception in Skript is that a block == an item type.
This could be due to the fact that Block's class info returns a string form of the item type of said block.
Ie:

send "Block: %block below player%"

returns:
"Block: stone"

Quite often I see people saving a block to a variable, setting a block to air, then trying to reset said block back to the variable, not realizing that the block in the variable is now air.
ex:

set {_block} to event-block #lets say its diamond ore
set event-block to air
wait 5 seconds
set event-block to {_block}

^^^ Here, the user assumes {_block} = diamond ore when in actuality it's the block object and the type is now air
I for one remember being frustrated with this when I first started using Skript, not understanding the block object.

In a case like this, the user should be using either type of block or block data of block
ex:

set {_b} to block data of event-block
set event-block to air
wait 5 seconds
set event-block to {_b}

As you can see in the code for the Block class info:

public String toString(final Block b, final int flags) {
return ItemType.toString(b, flags);
}

We are returning the ItemType as a String.

One proposal I have, is to have it actually return a block string, rather than an ItemType, ex:
(this is just an example of something that can return, that can be a little more detailed for the user)

Block[location=x:1,y:1,z:1,w:world,type=stone]

or maybe something like:

'stone' block at location 1, 1, 1 in 'world'

Understandably this could break a lot of scripts. For example, when a user is doing a string comparison, ie:

if "%block below player%" = "grass":

The user SHOULD be using type of block in this case, but due to the common misconception of a block object, and the fact the block's toString() method returns its item type as a string, a lot of Skripters misuse this.

I wanted to open this up for discussion, and see if maybe we can come up with a way to better use the block object in Skript.

@ShaneBeee ShaneBeee added enhancement Feature request, an issue about something that could be improved, or a PR improving something. dev needed labels Aug 16, 2020
@FranKusmiruk
Copy link
Member

FranKusmiruk commented Aug 17, 2020

I fully agree, users shouldn't think of blocks just as items since they are not, blocks are items placed in a location. I believe <type> at <x>, <y>, <z> is good enough of a representation without being too verbose. We could also mention the world in the string representation but in my opinion, it's rather redundant.

As for the string comparison, people shouldn't be doing that to begin with, we have a warning about it for that reason. While convenient in some rather specific cases, it is a misuse so I don't mind breaking some scripts as long as who did them realizes what they are doing wrong.

@ShaneBeee
Copy link
Contributor Author

We could also mention the world in the string representation but in my opinion, it's just redundant and verbose to have it.

I'm 50/50 on this. While I agree its mostly redundant, Im sure someone out there might find it useful.
But my heart wouldn't be broken if it wasn't there. For the most part users aren't sending a string of the block unless they're debugging, and if that is the case, Im sure they know which world they're in.
As long as we are representing the block as a block and not an item type, I think that's the main goal here.

As for the string comparison, people shouldn't be doing that to begin with, we have a warning about it for that reason. While convenient in some rather specific cases, it is a misuse so I don't mind breaking some scripts doing it as long as who did them realizes what they are doing wrong.

I 100% agree. Im sure ive done hacky string comparisons with blocks in the past, but most likely due to comparison issues with some blocks (which I think have all been fixed) .... at the end of the day, the user should really be using type of block for doing any type comparisons.
This would be something we gotta push on people!

@AlienDog117
Copy link

I think the world should be included because I can see how it may be useful. I also really like this idea, and I have had the exact scenario that you gave as an example! This thing helps me so much!

@ShaneBeee
Copy link
Contributor Author

ShaneBeee commented Aug 20, 2020

I think the world should be included because I can see how it may be useful.

Honestly I don't think it would hurt to have the world there. The more details the better.
Since this string return should mainly be used for debugging purposes, including world might be a good idea.
Especially if the block is saved in a variable.

ex:

send "Block: %block below player%"

could result in

Block: 'stone' at 3, 63, 3, 'world'

@fednelpat
Copy link
Contributor

fednelpat commented Aug 26, 2020

Yeah, I also feel like there should be examples in the skript docs that show how to correctly save block data to a variable to use in the future

@ShaneBeee ShaneBeee added the completed The issue has been fully resolved and the change will be in the next Skript update. label May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completed The issue has been fully resolved and the change will be in the next Skript update. enhancement Feature request, an issue about something that could be improved, or a PR improving something.
Projects
None yet
Development

No branches or pull requests

4 participants