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

EffSecDrop #6745

Closed
1 task done
TheAbsolutionism opened this issue May 30, 2024 · 11 comments
Closed
1 task done

EffSecDrop #6745

TheAbsolutionism opened this issue May 30, 2024 · 11 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

@TheAbsolutionism
Copy link

Suggestion

To add or change the drop effect to match the spawn effect.
With the Spawn effect, you have both
Effect
(spawn|summon) %entitytypes% [%directions% %locations%]
Effect Section

(spawn|summon) %entitytypes% [%directions% %locations%]:
    # entity expression

Im proposing for the drop effect to also have a section

drop %itemtypes/experiences% [%directions% %locations%] [(without velocity)]:
     # dropped entity expression

Why?

This would allow users to add/set NBT+values of the dropped entity with a higher sense of accuracy.
Following suit of the spawn effect, minimalizing the usage of the last spawned %entitytype%

Other

No response

Agreement

  • I have read the guidelines above and affirm I am following them with this suggestion.
@sovdeeth
Copy link
Member

sovdeeth commented May 30, 2024

I don't see the point of this. You can just spawn a dropped item with the spawn section and set the NBT of that just as easily via item of %entity%. What would this gain over that?

@NotSoDelayed
Copy link
Contributor

Concurrency — some might have some beefs with last dropped item expression due to this

@sovdeeth
Copy link
Member

Concurrency — some might have some beefs with last dropped item expression due to this

Skript does not run any code in parallel, so concurrency is not a concern. Also, you don't have to use last dropped item if you use the spawn section.

@TheAbsolutionism
Copy link
Author

I dont know if maybe I am just doing it wrong. But
https://github.com/SkriptLang/Skript/assets/82696841/daa9f0ec-e67f-423c-a581-771cbb62d121

@sovdeeth
Copy link
Member

I dont know if maybe I am just doing it wrong. But https://github.com/SkriptLang/Skript/assets/82696841/daa9f0ec-e67f-423c-a581-771cbb62d121

You need to use an item type, eg dropped stone

@TheAbsolutionism
Copy link
Author

@sovdeeth
Copy link
Member

Hmm I'll have to look into that tonight, it should be valid.

@sovdeeth
Copy link
Member

Looks like dropped items aren't spawnable through the normal methods, however, I think we should override the spawning method for DroppedItemData to allow it to be spawned normally via World#dropItem().

@ShaneBeee
Copy link
Contributor

Looks like dropped items aren't spawnable through the normal methods, however, I think we should override the spawning method for DroppedItemData to allow it to be spawned normally via World#dropItem().

I think this is an issue from my EntityData#canSpawn thing. I'm starting to regret that.

@sovdeeth
Copy link
Member

No, no, it's working properly! Bukkit doesn't allow you to spawn Item entities with World#spawn. I'm adding an override to the spawn method of its entityData so it can use dropItem right now :)

@sovdeeth sovdeeth added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label May 30, 2024
@sovdeeth
Copy link
Member

sovdeeth commented Jun 1, 2024

Spawning item entities is valid after 2.8.6, so I'm closing this for now. If there's some feature that you think a drop section would have over the spawn section, please reopen or make a new PR!

@sovdeeth sovdeeth closed this as completed Jun 1, 2024
@sovdeeth sovdeeth added the completed The issue has been fully resolved and the change will be in the next Skript update. label Jun 1, 2024
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