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

Value-getter return default value if the actual value is explicitly set to 0 #206

Closed
TBlueF opened this issue May 23, 2021 · 5 comments
Closed

Comments

@TBlueF
Copy link

TBlueF commented May 23, 2021

Version

dependencies {
    implementation 'org.spongepowered:configurate-hocon:4.1.1'
}

Description

Given:
A (hocon) configuration-file node has its value explicitly set to 0.
It got loaded using a HoconConfigurationLoader .
The value is retrieved as a float using a getter with a default-fallback argument (e.g. node.getFloat(1f))

Then:
That getter returns the default/fallback-value. (e.g. 1f)

Expected result:
That getter returns the explicitly set value: 0f

Extra info:
This only happens with float values.
This only happens if the node got loaded from a config-file.
I only tested the HOCON-loader.

Code Example

public static void main(String[] args) throws IOException {
	Path testFile = Path.of("testFile.conf").toAbsolutePath();
	Files.createDirectories(testFile.getParent());

	HoconConfigurationLoader loader = HoconConfigurationLoader
			.builder()
			.path(testFile)
			.build();

	ConfigurationNode originalNode = loader.createNode();
	originalNode.node("test").set(0f);

	loader.save(originalNode);

	System.out.println("## Original Node:");
	testNode(originalNode);

	ConfigurationNode loadedNode = loader.load();

	System.out.println("\n## Loaded Node:");
	testNode(loadedNode);
}

public static void testNode(ConfigurationNode node) {
	float result = node.node("test").getFloat();
	float resultWithDefault = node.node("test").getFloat(1f); // <- this returns 1f instead of 0f if the node got loaded

	System.out.println("Result: " + result);
	System.out.println("Result with default: " + resultWithDefault);
}

The console output of the above code:

## Original Node:
Result: 0.0
Result with default: 0.0

## Loaded Node:
Result: 0.0
Result with default: 1.0             <-- should be 0.0

The contents of the testFile.conf (after execution):

test=0
@TBlueF
Copy link
Author

TBlueF commented May 23, 2021

The same seems to happen using the GsonConfigurationLoader (using org.spongepowered:configurate-gson:4.1.1)

@TBlueF
Copy link
Author

TBlueF commented May 30, 2021

I tracked this issue down to be probably caused by this method returning false if the tested double is exactly 0.0.

Replacing:

if (!Double.isFinite(test)) { // NaN, ±inf

with something like:

if (test == 0.0 || !Double.isFinite(test)) { // zero, NaN, ±inf

should fix this :)

If you want, i can create a pull-request with this fix, but you'll be probably faster if you do it yourself :)

@amaranth
Copy link

amaranth commented Jun 9, 2021

Might want to stick a check in there for -0.0 too, although isFinite is doing Math.abs(d) <= Double.MAX_VALUE so I'm not sure why this would fail on zero to begin with. Did you try this suggested fix at all?

TBlueF added a commit to BlueMap-Minecraft/BlueMap that referenced this issue Jun 9, 2021
When that issue is fixed, this commit can be reverted.
@TBlueF
Copy link
Author

TBlueF commented Jun 9, 2021

@amaranth
Yes, i tested it successfully.
Double.isFinite(test) is negated (!Double.isFinite(test)) in that if-statement, so the extra check for 0 is needed :)

@zml2008
Copy link
Member

zml2008 commented Jun 11, 2021

I'm sorry it's taken a while to get to this issue, but it will be fixed in the release of 4.1.2, available in 4.1.2-SNAPSHOT dev builds within the next few minutes.

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