Comments Not Copied During Update

[creatorfromhell]

Hello,

I have recently started using BoostedYAML, and when testing around with the auto update function, I noticed that when BoostedYAML updates to the new defaults it doesn't include the comments attached to the new routes, and it also doesn't retain the spacing instead ignoring any spacing in the default file.

Version: 1.3.5

[dejvokep]

Hello,

Although the preservation of comments across updates is verified by automated tests, something else might have happened during updating which caused the issue you are experiencing.

Just to confirm, I checked and found no issues with v1.3.5:

YamlDocument.create(new File("old.yml"), Files.newInputStream(Paths.get("new.yml")),
                LoaderSettings.builder().setAutoUpdate(true).build(),
                UpdaterSettings.builder().setVersioning(new BasicVersioning("v")).setAutoSave(false).build()).save(new File("updated.yml"));

Current (to be updated) document (old.yml):

v: 1

# comment for a
a: "value 1"

Default document (new.yml):

v: 2

# comment for a
a: "value 1"

# comment for b
b: "value 2"

The resulting document (updated.yml):

v: '2'

# comment for a
a: value 1

# comment for b
b: value 2

Could you please provide your setup (with which the improper result was evaluated)?

Thank you. ~ dejvokep

[creatorfromhell]

Hello,

Although the preservation of comments across updates is verified by automated tests, something else might have happened during updating which caused the issue you are experiencing.

Just to confirm, I checked and found no issues with v1.3.5:

YamlDocument.create(new File("old.yml"), Files.newInputStream(Paths.get("new.yml")),
                LoaderSettings.builder().setAutoUpdate(true).build(),
                UpdaterSettings.builder().setVersioning(new BasicVersioning("v")).setAutoSave(false).build()).save(new File("updated.yml"));

Current (to be updated) document (old.yml):

v: 1

# comment for a
a: "value 1"

Default document (new.yml):

v: 2

# comment for a
a: "value 1"

# comment for b
b: "value 2"

The resulting document (updated.yml):

v: '2'

# comment for a
a: value 1

# comment for b
b: value 2

Could you please provide your setup (with which the improper result was evaluated)?

Thank you. ~ dejvokep

Mine is a little split up between multiple classes, but in essence it's:

YamlDocument.create(file, LoaderSettings.builder().setAutoUpdate(true).build(),
            UpdaterSettings.builder().setAutoSave(true).setVersioning(new BasicVersioning("Core.config-version")).build())

and the defaults file is, where SharedRegions should be added:

# The New Economy v0.1.2.0
# Author: creatorfromhell
# License: https://github.com/TheNewEconomy/EconomyCore/blob/main/license.md
Core:

  config-version: 2

  #Cracked Server? Set this to true. May cause data issues as spigot/paper UUIDs are unpredictable.
  #Do not change this unless you know what you're doing.
  Offline: false

  #Configurations relating to debugging
  Debugging:

    #The mode that debug should be in.
    #Modes: Off, Standard, Detailed, Developer
    Mode: "Off"

  #All configurations relating to regions/worlds.
  Region:

    #The region mode that TNE should be using for separating balances.
    #Valid modes: world, biome
    Mode: "world"

    #Should TNE separate account balances by region?
    #This will separate them based on the Mode configuration.
    MultiRegion: false

    #The name of the region to use if MultiRegion is set to false.
    #Setting to TNE_SYSTEM will let TNE decide the default region.
    DefaultRegion: "TNE_SYSTEM"

    #Should TNE automatically group the world's realms with the main overworld world.
    #Example: Setting this to true would make world, world_nether and world_the_end share balances automatically.
    GroupRealms: true

    #Regions that should have their economy disabled.
    SharedRegions:

      ExampleRegion: test

What is generated from the update:

# The New Economy v0.1.2.0
# Author: creatorfromhell
# License: https://github.com/TheNewEconomy/EconomyCore/blob/main/license.md
Core:

  config-version: '2'

  #Cracked Server? Set this to true. May cause data issues as spigot/paper UUIDs are unpredictable.
  #Do not change this unless you know what you're doing.
  Offline: false

  #Configurations relating to debugging
  Debugging:

    #The mode that debug should be in.
    #Modes: Off, Standard, Detailed, Developer
    Mode: Off

  #All configurations relating to regions/worlds.
  Region:

    #The region mode that TNE should be using for separating balances.
    #Valid modes: world, biome
    Mode: world

    #Should TNE separate account balances by region?
    #This will separate them based on the Mode configuration.
    MultiRegion: false

    #The name of the region to use if MultiRegion is set to false.
    #Setting to TNE_SYSTEM will let TNE decide the default region.
    DefaultRegion: TNE_SYSTEM

    #Should TNE automatically group the world's realms with the main overworld world.
    #Example: Setting this to true would make world, world_nether and world_the_end share balances automatically.
    GroupRealms: true
    SharedRegions:

      ExampleRegion: test

[dejvokep]

Good day,

Could you please confirm that the original document (with version 1) does not contain the SharedRegions section? Could you please also present it here in order to better evaluate your case?

Thanks. ~ dejvokep

[creatorfromhell]

#Regions that should have their economy disabled.
    SharedRegions:

      ExampleRegion: test

Sure I left out the config, because it's just the version 2 default without the SharedRegions config, but this is it:

# The New Economy v0.1.2.0
# Author: creatorfromhell
# License: https://github.com/TheNewEconomy/EconomyCore/blob/main/license.md
Core:

  config-version: '1'

  #Cracked Server? Set this to true. May cause data issues as spigot/paper UUIDs are unpredictable.
  #Do not change this unless you know what you're doing.
  Offline: false

  #Configurations relating to debugging
  Debugging:

    #The mode that debug should be in.
    #Modes: Off, Standard, Detailed, Developer
    Mode: Off

  #All configurations relating to regions/worlds.
  Region:

    #The region mode that TNE should be using for separating balances.
    #Valid modes: world, biome
    Mode: world

    #Should TNE separate account balances by region?
    #This will separate them based on the Mode configuration.
    MultiRegion: false

    #The name of the region to use if MultiRegion is set to false.
    #Setting to TNE_SYSTEM will let TNE decide the default region.
    DefaultRegion: TNE_SYSTEM

    #Should TNE automatically group the world's realms with the main overworld world.
    #Example: Setting this to true would make world, world_nether and world_the_end share balances automatically.
    GroupRealms: true

  #All configurations relating to the server in general.
  Server:

    #The geyser prefix for the server.
    Geyser: .

    #Whether the ServerID config should be random on each startup. This could impact syncing on
    #restarts, but will not impact anything significant.
    RandomUUID: false

    #Whether to import exist item currencies into a player's balance when they join for the first time.
    ImportItems: true

    #Should TNE disable mob drops that are valid item currencies?
    MobDrop: true

    #Should experience gaining be disabled? This will help for servers that use Experience as currency.
    ExperienceGain: false

    #The name of this server for data-related purposes. Max length is 100 characters.
    Name: Main Server

    #All configurations relating to the server's economy account.
    Account:

      #Enable the server account?
      Enabled: true

      #The name of the server account. Max length is 100 characters.
      Name: Server_Account

      #The starting balance for the server account.
      Balance: 500

  #All configurations relating to TNE commands
  Commands:

    #Configuration if money action commands, such as give/take/set require individual permissions.
    LimitCurrency: false

    #Configurations relating to the money top command.
    Top:

      #Should balances in /baltop be formatted?
      Format: true

      #How often should the data used for /baltop be refreshed?
      #This is in seconds.
      Refresh: 1200

      #A list of values to use to exclude certain users from baltop if the username contains these values.
      Exclusions:
      - ^town-.*
      - ^nation-.*
      - ^faction-.*
      - ^towny-.*

    #Extra configurations regarding the pay command
    Pay:

      #Can players use /pay on offline players?
      Offline: true

      #How close to another player someone must be, in blocks, in order to use /pay on them. Set to 0 to disable.
      Radius: 0

  #All configurations relating to update checking
  Update:

    #Should TNE check for updates?
    Check: true

    #Should TNE notify any users with the tne.admin node of TNE updates on join?
    Notify: true

  #All configurations relating to the transaction system.
  Transactions:

    #The time format to use when displaying transaction history data.
    Format: M, d y

    #The timezone to use for transactions.
    Timezone: US/Eastern

    #Configurations relating to tracking large transactions
    Tracking:

      #If large transaction tracking is enabled or not.
      Enabled: true

      #The threshold to mark a transaction for tracking.
      Amount: '400'

    #Configurations relating to Transaction History
    History:

      #How often should the data used for /transaction list be refreshed?
      #This is in seconds.
      Refresh: 1200

    #Configurations relating to conversion transactions.
    Conversion:

      #Tax-related configurations.
      Tax:

        #If this tax is enabled.
        Enabled: false

        #The tax to apply to a conversion transaction.
        #The tax may also be a percentage. For percent follow number by the % symbol, i.e 10%
        Rate: 0.0

    #Configurations relating to desposit transactions.
    Deposit:

      #Tax-related configurations.
      Tax:

        #If this tax is enabled.
        Enabled: false

        #The tax to apply to a deposit transaction.
        #The tax may also be a percentage. For percent follow number by the % symbol, i.e 10%
        Rate: 0.0

    #Configurations relating to pay transactions.
    Pay:

      #Tax-related configurations.
      Tax:

        #If this tax is enabled.
        Enabled: false

        #The tax to apply to the sender
        #The tax may also be a percentage. For percent follow number by the % symbol, i.e 10%
        Sender: 0.0

        #The tax to apply to the receiver
        #The tax may also be a percentage. For percent follow number by the % symbol, i.e 10%
        Receiver: 0.0

    #Configurations relating to withdraw transactions.
    Withdraw:

      #Tax-related configurations.
      Tax:

        #If this tax is enabled.
        Enabled: false

        #The tax to apply to a withdrawal transaction.
        #The tax may also be a percentage. For percent follow number by the % symbol, i.e 10%
        Rate: 0.0
  #Don't modify unless you know what you're doing.
  ServerID: 8683be6c-5642-46f0-9ec5-969e4493b71f

[dejvokep]

Hello,

Thank you, was able to replicate the issue just perfectly. A fix has been merged successfully under #33 and a new release will be created soon. In the meanwhile, if you wish to access it immediately, clone the repository and build the artifact yourself by running mvn install. Thanks for reporting!

Should there be any further issue or questions, please do not hesitate and let me know. ~ dejvokep

[creatorfromhell]

Hello,

Thank you, was able to replicate the issue just perfectly. A fix has been merged successfully under #33 and a new release will be created soon. In the meanwhile, if you wish to access it immediately, clone the repository and build the artifact yourself by running mvn install. Thanks for reporting!

Should there be any further issue or questions, please do not hesitate and let me know. ~ dejvokep

Cheers

[creatorfromhell]

Hello,

Thank you, was able to replicate the issue just perfectly. A fix has been merged successfully under #33 and a new release will be created soon. In the meanwhile, if you wish to access it immediately, clone the repository and build the artifact yourself by running mvn install. Thanks for reporting!

Should there be any further issue or questions, please do not hesitate and let me know. ~ dejvokep

Was wondering if there was an update on this?

[dejvokep]

Hi, yes, I'll make sure to push a new release today.

[dejvokep]

Hi, yes, I'll make sure to push a new release today.

Sorry for the delay, waited for one more contribution and then I couldn't find the time necessary to check everything once again and publish a new release. At the time of writing, the v1.3.7 release is available for download from Maven Central.

[GRX005]

Hi, I'm using the latest v1.3.7 of the lib, but the issue still seems to be there.

I put this new section in the config file and updated the file version, but it's only putting the 'Rewards: null' part, and not the comments. If I delete the config file, only then it will put the comments there. @dejvokep

Code that loads/creates the config:

[dejvokep]

Hi, @GRX005,

This is not an issue related to the newest release, nor to the library itself. The underlying SnakeYAML Engine parses trailing comments (at the end of the file, if any) as the "footer" of the document, meaning these do not belong to the Rewards block, but rather to the document itself.

Therefore, as the document (root) is not a new node, it's skipped and already existing content (including comments) is preserved, as is the case with any sections which are present in both the document and the defaults during updating.

To verify and confirm this behaviour, load the defaults (the new version of the document) and you'll be able to access the trailing comments by:

Comments.get(document, NodeRole.VALUE, Comments.Position.AFTER);

Because of this and other limitations, I strongly advise against putting any comments belonging to a node after it's content (represented by Position.AFTER).

Have a great day :) ~ David K.