[Drilldown Pies] Info is mixed up despite values being unique

[Andre601]

I encounter a rather weird behaviour, where a Drilldown Pie displays "1.16.5" in the "1.12.x" section and "1.12.2" in "1.16.x"

The logic behind making this Pie doesn't suggest any failure on my end, so I can only assume bstats somehow mixing values up? Any help in solving this would be appreciated.

To the infos: I use the BungeeCord version of the Bstats library and have the following code used to apply a custom Drilldown Pie.

    // The void is from an interface as I also have a Velocity class.
    @Override
    public void loadMetrics(){
        Metrics metrics = new Metrics(this, 10340);

        metrics.addCustomChart(new DrilldownPie("allowed_versions", () -> {
            Map<String, Map<String, Integer>> map = new HashMap<>();
            Map<String, Integer> entry = new HashMap<>();

            List<Integer> protocolVersions = getConfigHandler().getIntList("Protocol", "Versions");
            if(protocolVersions.isEmpty()){
                String unknown = ProtocolVersion.getFriendlyName(0);
                entry.put(unknown, 1);
                map.put("other", entry);

                return map;
            }

            for(int version : protocolVersions){
                String major = ProtocolVersion.getMajor(version);
                String name = ProtocolVersion.getFriendlyName(version);

                entry.put(name, 1);
                if(major.equalsIgnoreCase("?")){
                    map.put("other", entry);
                }else{
                    map.put(major, entry);
                }
            }

            return map;
        }));
    }

The ProtocolVersion is a enum with following content:

/*
 * Copyright 2020 - 2021 Andre601
 *
 * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
 * documentation files (the "Software"), to deal in the Software without restriction, including without limitation
 * the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software,
 * and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
 *
 * The above copyright notice and this permission notice shall be included in all copies or substantial
 * portions of the Software.
 *
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
 * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
 * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
 * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE
 * OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 */

package com.andre601.oneversionremake.core.enums;

import java.util.Comparator;
import java.util.List;
import java.util.stream.Collectors;

public enum ProtocolVersion{
    MC_1_16_5(754, "1.16.5", "1.16.x"),
    MC_1_16_3(753, "1.16.3", "1.16.x"),
    MC_1_16_2(751, "1.16.2", "1.16.x"),
    MC_1_16_1(736, "1.16.1", "1.16.x"),
    MC_1_16  (735, "1.16",   "1.16.x"),
    MC_1_15_2(578, "1.15.2", "1.15.x"),
    MC_1_15_1(575, "1.15.1", "1.15.x"),
    MC_1_15  (573, "1.15",   "1.15.x"),
    MC_1_14_4(498, "1.14.4", "1.14.x"),
    MC_1_14_3(490, "1.14.3", "1.14.x"),
    MC_1_14_2(485, "1.14.2", "1.14.x"),
    MC_1_14_1(480, "1.14.1", "1.14.x"),
    MC_1_14  (477, "1.14",   "1.14.x"),
    MC_1_13_2(404, "1.13.2", "1.13.x"),
    MC_1_13_1(401, "1.13.1", "1.13.x"),
    MC_1_13  (393, "1.13",   "1.13.x"),
    MC_1_12_2(340, "1.12.2", "1.12.x"),
    MC_1_12_1(338, "1.12.1", "1.12.x"),
    MC_1_12  (335, "1.12",   "1.12.x"),
    MC_1_11_2(316, "1.11.2", "1.11.x"),
    MC_1_11  (315, "1.11",   "1.11.x"),
    MC_1_10_2(210, "1.10.2", "1.10.2"),
    MC_1_9_4 (110, "1.9.4",  "1.9.x"),
    MC_1_9_2 (109, "1.9.2",  "1.9.x"),
    MC_1_9_1 (108, "1.9.1",  "1.9.x"),
    MC_1_9   (107, "1.9",    "1.9.x"),
    MC_1_8_9 (47,  "1.8.9",  "1.8.9"),

    // In case none of the above versions match
    UNKNOWN  (0,   "?", "?");

    private final int protocol;
    private final String friendlyName;
    private final String major;

    ProtocolVersion(int protocol, String friendlyName, String major){
        this.protocol = protocol;
        this.friendlyName = friendlyName;
        this.major = major;
    }

    public static String getFriendlyName(int protocol){
        for(ProtocolVersion version : values()){
            if(version.getProtocol() == protocol)
                return version.getFriendlyName();
        }

        return UNKNOWN.getFriendlyName();
    }

    public static String getFriendlyNames(List<Integer> protocols, boolean majorOnly){
        if(majorOnly){
            return protocols.stream()
                    .sorted(Comparator.reverseOrder())
                    .map(ProtocolVersion::getMajor)
                    .distinct()
                    .collect(Collectors.joining(", "));
        }

        return protocols.stream()
                .sorted(Comparator.reverseOrder())
                .map(ProtocolVersion::getFriendlyName)
                .collect(Collectors.joining(", "));
    }

    public static String getMajor(int protocol){
        for(ProtocolVersion version : values()){
            if(version.getProtocol() == protocol)
                return version.getMajor();
        }

        return UNKNOWN.getMajor();
    }

    private int getProtocol(){
        return protocol;
    }

    private String getFriendlyName(){
        return friendlyName;
    }

    private String getMajor(){
        return major;
    }
}

As you can see do both the getMajor and getFriendlyName method use the provided integer to determine what String to return. getMajor is used for the parent Map while the getFriendlyName version is used for the child Map (The detailed view).

The issue now is, like mentioned before, that 1.16.x has 1.12.2 and 1.12.x has 1.16.5 as entries, which should never be the case by the above logic as each separate version integer is unique per version and would therefore not result in this.

The only other case I could imagine is that the maps are somehow mixed up before the info is handled by Bstats to then send. Maybe a LinkedHashMap could be a solution? Other than that can I not think of any real idea what else could cause this and can only assume something on the BStats-end to cause this.

Here's a screenshot of what I mean:

[Bastian]

I think your problem is, that you re-use the entry map if there is more than one element in the protocolVersions list.

In the end, all the entries in your map have the same value. E.g.

Map<String, Map<String, Integer>> map = new HashMap<>();
Map<String, Integer> entry = new HashMap<>();

entry.put("one", 1);
map.put("x", entry);
entry.put("two", 2);
map.put("y", entry);

will not produce

{
  "x": {
    "one": 1
  },
  "y": {
    "two": 2
  }
}

but

{
  "x": {
    "one": 1,
    "two": 2
  },
  "y": {
    "one": 1,
    "two": 2
  }
}

So your code should probably look like this:

@Override
public void loadMetrics(){
    Metrics metrics = new Metrics(this, 10340);

    metrics.addCustomChart(new DrilldownPie("allowed_versions", () -> {
        Map<String, Map<String, Integer>> map = new HashMap<>();

        List<Integer> protocolVersions = getConfigHandler().getIntList("Protocol", "Versions");
        if(protocolVersions.isEmpty()){
            String unknown = ProtocolVersion.getFriendlyName(0);

            Map<String, Integer> entry = new HashMap<>();

            entry.put(unknown, 1);
            map.put("other", entry);

            return map;
        }

        for(int version : protocolVersions){
            String major = ProtocolVersion.getMajor(version);
            String name = ProtocolVersion.getFriendlyName(version);

           Map<String, Integer> entry = new HashMap<>();

            entry.put(name, 1);
            if(major.equalsIgnoreCase("?")){
                map.put("other", entry);
            }else{
                map.put(major, entry);
            }
        }

        return map;
    }));
}

This is ofc only the case if getConfigHandler().getIntList("Protocol", "Versions"); ever returns a list with more than 1 element. I'm not familiar with your project/plugin, so if this is not the case, please tell me and I'll look deeper into it :)

[Andre601]

getIntList does, as the name suggests (And as shown in the code) return a List<Integer> so there's always a chance of having more than 1 entry in the returned list,

Anyway, thanks for this info. I now implemented the change and hope it will work.

[Andre601]

Also, wouldn't your example produce this?

{
  "x": {
    "one": 1
  },
  "two": {
    "one": 1,
    "two": 2
  }
}

Since your example does add the "two" after the map was already added to x (And I'm not sure but I don't believe java updates instances of something inside a map unless you directly call it)

[Bastian]

No, because the entry variable is just a reference to a mutable object. You are not actually putting the content of the entry in the map map but just a reference to the entry map. If you modify the entry map after putting it into map, it will also change its content inside of map.

I recommend to take a look at https://java-programming.mooc.fi/part-5/3-primitive-and-reference-variables if you are unfamilar with the concept of references.