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

Default value being null on EntryData causes the default value to always throw #7468

Open
1 task done
TheLimeGlass opened this issue Jan 18, 2025 · 4 comments
Open
1 task done
Assignees

Comments

@TheLimeGlass
Copy link
Contributor

TheLimeGlass commented Jan 18, 2025

Skript/Server Version

Server Version: 1.21.4-116-e150ffd (MC: 1.21.4)
Skript Version: 2.10.0-nightly-d3dd04165 (skriptlang-nightly)
Installed Skript Addons: None
Installed dependencies: None

Bug Description

When registering the default value as null

EntryValidator.builder()
	.addEntry("example", null, true)
	.build();

and defining true to return the default.

container.get("example", true);

Skript will error

[12:36:29 ERROR]: #!#! Stack trace:
[12:36:29 ERROR]: #!#! Caused by: java.lang.RuntimeException: Null value for asserted non-null value
[12:36:29 ERROR]: #!#!     at Skript-2.10.0.jar//org.skriptlang.skript.lang.entry.EntryContainer.get(EntryContainer.java:78)
[12:36:29 ERROR]: #!#!     at Skript-2.10.0.jar//ch.njol.skript.structures.StructAutoReload.init(StructAutoReload.java:113)
[12:36:29 ERROR]: #!#!     at Skript-2.10.0.jar//org.skriptlang.skript.lang.structure.Structure.init(Structure.java:124)
[12:36:29 ERROR]: #!#!     at Skript-2.10.0.jar//ch.njol.skript.lang.SkriptParser.parse(SkriptParser.java:241)
[12:36:29 ERROR]: #!#!     at Skript-2.10.0.jar//ch.njol.skript.lang.SkriptParser.parseStatic(SkriptParser.java:175)
[12:36:29 ERROR]: #!#!     at Skript-2.10.0.jar//org.skriptlang.skript.lang.structure.Structure.parse(Structure.java:220)
[12:36:29 ERROR]: #!#!     at Skript-2.10.0.jar//org.skriptlang.skript.lang.structure.Structure.parse(Structure.java:203)
[12:36:29 ERROR]: #!#!     at Skript-2.10.0.jar//ch.njol.skript.ScriptLoader.loadScript(ScriptLoader.java:664)
[12:36:29 ERROR]: #!#!     at Skript-2.10.0.jar//ch.njol.skript.ScriptLoader.lambda$loadScripts$4(ScriptLoader.java:476)
[12:36:29 ERROR]: #!#!     at Skript-2.10.0.jar//ch.njol.skript.ScriptLoader.lambda$makeFuture$2(ScriptLoader.java:393)
[12:36:29 ERROR]: #!#!     at Skript-2.10.0.jar//ch.njol.skript.ScriptLoader$AsyncLoaderThread.run(ScriptLoader.java:343)
[12:36:29 ERROR]: #!#! 

Expected Behavior

Skript should not error if the default value is null itself. Skript should simply return null as it's the expected return.
Using an object or Optionals can fix this from happening.

Using

container.getOptional("example", true);

Causes the same error to occur.

Agreement

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

You can't use null as a default value. It doesn't really make sense. What you want to use is getOptional.

@TheLimeGlass
Copy link
Contributor Author

TheLimeGlass commented Jan 18, 2025

You can't use null as a default value. It doesn't really make sense. What you want to use is getOptional.

The defaultValue is stated nullable, and there is no constructor parameters to not provide a default value. I don't want one, that's why I put null.

I want it to return null if there is no value present.

getOptional also throws the same error.

@Fusezion
Copy link
Contributor

Fusezion commented Jan 23, 2025

I would like to see changes made to EntryContainer#get method as well, an example being the screenshot below
first reload was me using block at player second is block at block, first reload works fine causes no stack traces

However using block at block this reloads "fine" but sends a stacktrace saying a null value was received I personally hate this since a developer has no way to handle what a user provides to them enforcing all systems to use a getOptional is in highly poor taste and why my issue #6028 was even made.

The entry itself is required, I see no reason on a dev end to use getOptional in place of it

A user as I mentioned before can still pass anything they want into that field (which isn't Object.class it's Block.class) and it'll be parsed as if it's fine, developers shouldn't be forced to use getOptional and then follow it up with a LiteralUtils.defendExpression()

The broadcast for "hasBlock was a hasEntry usage seeing if it was removed from handled entries if it was invalid (it isn't) this leads to no way for a developer to safely and accurate error on parse time on invalid entries we're just expected to accept it

Image

@APickledWalrus APickledWalrus self-assigned this Jan 23, 2025
@APickledWalrus
Copy link
Member

You can't use null as a default value. It doesn't really make sense. What you want to use is getOptional.

The defaultValue is stated nullable, and there is no constructor parameters to not provide a default value. I don't want one, that's why I put null.

I want it to return null if there is no value present.

getOptional also throws the same error.

If you want null, use getOptional. It wouldn't be possible to get the same error from it, and if you are, can you send a stack trace?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants