Skript icon indicating copy to clipboard operation
Skript copied to clipboard

Mod Material support, for Hybrid Server

Open 0XPYEX0 opened this issue 1 year ago • 26 comments
trafficstars

Description

As title, support for items which added by mods For PR: #7121 ①

on enchant:
    if event-item is (cataclysm's the incinerator): # The item ID
        send "success" to player

b258fdd942e5bf2210428d7b21cdb3d9

on right click:
    send "%type of tool of player%" to player

63134e0a5c43b268706222ad750c48d0 It's an old screenshot. Now it's quack's abacus

The pattern just like: mod:an_item_name_with_line (parsed to alias)→ mod's an item name with line And vanilla aliases are not changed. minecraft:dirtdirt


Target Minecraft Versions: any Requirements: none Related Issues: #4051 #4678 #4778 #6503

0XPYEX0 avatar Oct 02 '24 16:10 0XPYEX0

I strongly recommend to approve this PR because:

  1. in future minecraft versions, maybe there will be a way to directly add blocks/items to minecraft by datapack. then the namespace may not be minecraft. but currently Skript doesn't support to create aliases for items which do not use minecraft as their namespace.
  2. as the 1st opinion said, if this PR is approved, then Hybird servers will also benefit from this, which could let modded items being added to Skript aliases.
  3. this PR doesn't break any Skript's current core things, but increased scalability.
  4. there are a lot of people want this too, such as https://forums.skunity.com/threads/14952/, https://forums.skunity.com/threads/9848/, https://forums.skunity.com/threads/14457/, etc. if you google "Skript Mod Item" or something similiar, you could find a lot more.

i know that Skript doesn't support Hybird servers, but this PR is intended to improve the scalability of Skript, not just for Hybird servers.

(ref: https://github.com/SkriptLang/Skript/pull/7121#issuecomment-2385658541)

If we check it, Skript cannot parse aliases of mod items (In Hybrid Server

(ref: https://github.com/SkriptLang/Skript/pull/7121)

good job! your unrealized plan in the previous PR was finally realized, while ensuring the core functions of Skript.

BredyAK avatar Oct 02 '24 17:10 BredyAK

i just did some tests, no worry, all of them are runned as expected:

  1. no matter what the option load default aliases in Skript's config file is set (true or false), it doesn't affect vanilla or mod's aliases' generation.
  2. when i execute !give cataclysm's the incinerator to player, i successfully get the expected item, and its id is cataclysm:the_incinerator.
  3. when i execute !give diamond chestplate to player, i successfully get the expected item, and its id is minecraft:diamond_chestplate.

this is an epic PR for me, thanks! this is why i love Skript's community.

BredyAK avatar Oct 02 '24 18:10 BredyAK

Hey guys, maybe we could do like this? image I add a of expression, and make it looks like quack mod's abacus or abacus of mod quack Should it be done or not?

@sovdeeth @Efnilite

0XPYEX0 avatar Oct 02 '24 18:10 0XPYEX0

Hey guys, maybe we could do like this?

~~I'm not a fan of the |s stuff~~, but i suppose mod's x and x from mod is ok actually ignore the first part

sovdeeth avatar Oct 02 '24 18:10 sovdeeth

Hey guys, maybe we could do like this?

I'm not a fan of the |s stuff, but i suppose mod's x and x from mod is ok

Just like abacus from mod quack, right? Not of ?

0XPYEX0 avatar Oct 02 '24 18:10 0XPYEX0

Hey guys, maybe we could do like this?

I'm not a fan of the |s stuff, but i suppose mod's x and x from mod is ok

Just like abacus from mod quack, right? Not of ?

I'd like <item> from <mod>, so abacus from quark

sovdeeth avatar Oct 02 '24 18:10 sovdeeth

Hey guys, maybe we could do like this?

I'm not a fan of the |s stuff

Actually idk what is it, but there was a ¦s before, so I kept it

0XPYEX0 avatar Oct 02 '24 18:10 0XPYEX0

Hey guys, maybe we could do like this?

I'm not a fan of the |s stuff

Actually idk what is it, but there was a ¦s before, so I kept it

ignore that message, it's fine to have

sovdeeth avatar Oct 02 '24 18:10 sovdeeth

c1644026a8294ce6503dc6fcdfb2bedc Successfully to get cataclysm:the_incinerator

0XPYEX0 avatar Oct 02 '24 18:10 0XPYEX0

I'm a bit concerned on how many aliases this will add if it's 3 per modded item, skript already suffers enough from how many they just add.

Fusezion avatar Oct 02 '24 19:10 Fusezion

I'm a bit concerned on how many aliases this will add if it's 3 per modded item, skript already suffers enough from how many they just add.

one point: they are not generated for everyone. with this PR, it will just create aliases for things that ALREADY in your server. if you don't have mod items (like vanilla server), it won't affect you at all.

BredyAK avatar Oct 02 '24 19:10 BredyAK

I'm a bit concerned on how many aliases this will add if it's 3 per modded item, skript already suffers enough from how many they just add.

Not 3 but 2. And this is not the reason to make a language not support mod items. That's not fair😰 If you are running a vanilla server, this will not affect you anymore If you are running a Hybrid server, this should be your good news 😊

0XPYEX0 avatar Oct 02 '24 19:10 0XPYEX0

I'm a bit concerned on how many aliases this will add if it's 3 per modded item, skript already suffers enough from how many they just add.

well it's 4 per item, vs 2 per item for the vanilla items even with 10,000 materials, that's only 40,000 aliases, which is pretty cheap The thing that makes aliases take so long is having hundreds or thousands of aliases for a single item, like stairs.

sovdeeth avatar Oct 02 '24 19:10 sovdeeth

I'm a bit concerned on how many aliases this will add if it's 3 per modded item, skript already suffers enough from how many they just add.

here is what this PR do:

  1. if you are a vanilla server, then just ignore this PR because it take no effect of your server.
  2. if you are a hybird server, then this PR will make Skript intelligently generate the aliases it needed. not only all the vanilla aliases will be kept, but also help you generate the extra aliases such as created by your mods, etc.

in future, maybe Mojang will provide a way to let creators add blocks/items/etc. to Minecraft by simply using datapakcs (like Bedrock Edition's behavior pack + resource pack), then creators could use their own nameplaces (not minceraft:), Skript will become more scalable then.

BredyAK avatar Oct 02 '24 19:10 BredyAK

I'm a bit concerned on how many aliases this will add if it's 3 per modded item, skript already suffers enough from how many they just add.

And this will be made less bad by #7084 :)

Efnilite avatar Oct 02 '24 19:10 Efnilite

one point: they are not generated for everyone. with this PR, it will just create aliases for things that ALREADY in your server. if you don't have mod items (like vanilla server), it won't affect you at all.

Thank but I'm aware I know what this PR does and I'm aware what I said

Not 3 but 2. And this is not the reason to make a language not support mod items. That's not fair If you are running a vanilla server, this will not affect you anymore If you are running a Hybrid server, this is your good news

Must of missed something, thought it was 3. I never said we should scrap this whole thing just because of that. It's just a fact skript still by default registers 233k in their own in addition to this

well it's 4 per item, vs 2 per item for the vanilla items even with 10,000 materials, that's only 40,000 aliases, which is pretty cheap The thing that makes aliases take so long is having hundreds or thousands of aliases for a single item, like stairs.

Wait wait it's 4 now? And no it really isn't cheap for 40k that's still a major concern granted it should be rare for anyone to reach 10k items, so I'm fine with stepping down on this but none the less it's concerning.

If anything a config option should still be added similar to vanilla minecraft items but only accept namespace key path and remove x's item and item from x

Fusezion avatar Oct 02 '24 19:10 Fusezion

well it's 4 per item, vs 2 per item for the vanilla items even with 10,000 materials, that's only 40,000 aliases, which is pretty cheap The thing that makes aliases take so long is having hundreds or thousands of aliases for a single item, like stairs.

Wait wait it's 4 now? And no it really isn't cheap for 40k that's still a major concern granted it should be rare for anyone to reach 10k items, so I'm fine with stepping down on this but none the less it's concerning.

40k is very cheap for 10k items, honestly. The benefit from having basic grammatical aliases far outweighs the slight reduction in alias count, imo. and it's 4 because of plurality, the |s adds a second plural alias, so vanilla is 2 and modded is 4

If anything a config option should still be added similar to vanilla minecraft items but only accept namespace key path and remove x's item and item from x

There is no such config option for vanilla items that does that, though.

and RE: the 233k current ones, those will not be included by default in 2.10, only the ~4000 auto generated ones for the ~2000 items minecraft has

sovdeeth avatar Oct 02 '24 19:10 sovdeeth

idk if there is a need to add a config option as @Fusezion said, but i could post my server log for reference:

[02:55:01 INFO]: [Skript] You're currently running a custom Skript version. No updates will be automatically installed.
[02:55:10 INFO]: [Skript] Loaded 260276 aliases in 8747ms

my server has 260000+ aliases and it loaded for 8747ms.

BredyAK avatar Oct 02 '24 19:10 BredyAK

I don't see a need for a config option, but I do want to ask whether we should add a warning when running on a modded server? We don't support modded platforms, so it is probably not a good idea for people to be using skript to build their modded servers. I know people will do it anyway, but I'd love to at least make it clear they are on their own.

Pikachu920 avatar Oct 02 '24 19:10 Pikachu920

I don't see a need for a config option, but I do want to ask whether we should add a warning when running on a modded server? We don't support modded platforms, so it is probably not a good idea for people to be using skript to build their modded servers. I know people will do it anyway, but I'd love to at least make it clear they are on their own.

i don't think there is a need to add warnings... because it doesn't affect vanilla server at all. and you are using hybird server, how couldn't you know the warning things?

btw,

We don't support modded platforms

this is not a thing only about modded platforms. i could use my reply above:

I'm a bit concerned on how many aliases this will add if it's 3 per modded item, skript already suffers enough from how many they just add.

here is what this PR do:

  1. if you are a vanilla server, then just ignore this PR because it take no effect of your server.
  2. if you are a hybird server, then this PR will make Skript intelligently generate the aliases it needed. not only all the vanilla aliases will be kept, but also help you generate the extra aliases such as created by your mods, etc.

in future, maybe Mojang will provide a way to let creators add blocks/items/etc. to Minecraft by simply using datapakcs (like Bedrock Edition's behavior pack + resource pack), then creators could use their own nameplaces (not minceraft:), Skript will become more scalable then.

BredyAK avatar Oct 02 '24 19:10 BredyAK

I don't see a need for a config option, but I do want to ask whether we should add a warning when running on a modded server? We don't support modded platforms, so it is probably not a good idea for people to be using skript to build their modded servers. I know people will do it anyway, but I'd love to at least make it clear they are on their own.

I feel like we should, because some modded platforms mess with some internal code so yeah, and plus if people go to open an issue about it, they may see that yellow warning message and not do it

EquipableMC avatar Oct 02 '24 20:10 EquipableMC

I'm neutral. It doesn't matter if we warn them or not.

0XPYEX0 avatar Oct 03 '24 07:10 0XPYEX0

I am in favor of this pull request. While it's true that Skript doesn't officially support Hybrid servers, a significant number of users still use it with Hybrid servers. Therefore, I believe it's highly beneficial to merge this.

MSCMDD avatar Oct 03 '24 09:10 MSCMDD

currently it doesn't support mod's entity type, maybe it will be better if this PR makes Skript support mod entities, just like using the command below to summon a mod entity:

!summon aether's blue swet at player's location

thank you for bringing this epic PR!

BredyAK avatar Oct 03 '24 16:10 BredyAK

currently it doesn't support mod's entity type, maybe it will be better if this PR makes Skript support mod entities, just like using the command below to summon a mod entity:

!summon aether's blue swet at player's location

thank you for bringing this epic PR!

That's out of scope for this PR, and probably out of scope in general for skript at the current moment, since it would involve adding entitydatas.

sovdeeth avatar Oct 03 '24 16:10 sovdeeth

!summon aether's blue swet at player's location

not planned now

0XPYEX0 avatar Oct 03 '24 16:10 0XPYEX0

Waiting for merging 🤔

0XPYEX0 avatar Oct 26 '24 10:10 0XPYEX0

How about the warnings🤔 Will this be too harsh?😰

0XPYEX0 avatar Oct 27 '24 16:10 0XPYEX0