SpongePowered / Configurate

A simple configuration library for Java applications providing a node structure, a variety of formats, and tools for transformation
https://configurate.aoeu.xyz
Apache License 2.0
373 stars 66 forks source link

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

Closed TBlueF closed 3 years ago

TBlueF commented 3 years ago

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 commented 3 years ago

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

TBlueF commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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.