Fractal Softworks Forum

Please login or register.

Login with username, password and session length
Advanced search  

News:

Starsector 0.97a is out! (02/02/24); New blog post: Simulator Enhancements (03/13/24)

Author Topic: [0.8.1a] Merging CSVs with different columns loses data  (Read 2302 times)

Histidine

  • Admiral
  • *****
  • Posts: 4661
    • View Profile
    • GitHub profile
[0.8.1a] Merging CSVs with different columns loses data
« on: May 13, 2018, 06:50:36 AM »

(bumped on 22 Jul 2020)

Short description
CSV 1 contains columns A (the id column), B, C. It is in the "master" mod.
CSV 2 contains columns A and D. It is in a second mod.
After merging the two files, any information in columns B and C is lost for any rows defined in CSV 2, even though CSV 1 has that information.

Example
Spoiler
CSV 1:
Code: csv
animal,legs,size
dog,4,medium
cat,4,small
spider,8,small
t-rex,2,large

CSV 2:
Code: csv
animal,eggs
dog,no
cat,no
parrot,yes

Test script:
Code: java
package data.scripts.world;

import com.fs.starfarer.api.Global;
import java.util.Iterator;
import org.apache.log4j.Logger;
import org.json.JSONArray;
import org.json.JSONObject;

public class TestScript {

public static Logger log = Global.getLogger(TestScript.class);
public static String testFile = "test_csv.csv";

public static void execute()
{
try {
JSONArray csv = Global.getSettings().getMergedSpreadsheetDataForMod("animal", testFile, "histidine_test");
for(int x = 0; x < csv.length(); x++)
{
JSONObject row = csv.getJSONObject(x);
log.info("Loading data for animal: " + row.getString("animal"));
log.info("\tColumn count: " + row.names().length());
Iterator columns = row.keys();
while( columns.hasNext() ) {
String column = (String)columns.next();
if (column.equals("fs_rowSource")) continue;
if (column.equals("animal")) continue;
log.info("\tValue of '" + column + "': " + row.getString(column));
}
}
} catch (Exception ex) {
log.error(ex);
}
}
}

Output:
Code: txt
2112180 [Thread-4] INFO  org.lazywizard.console.Console  - > runcode data.scripts.world.TestScript.execute()
2112994 [Thread-4] INFO  com.fs.starfarer.loading.LoadingUtils  - Loading CSV data from [DIRECTORY: D:\Program Files\Fractal Softworks\Starsector\starsector-core\..\mods\testmod]
2112995 [Thread-4] INFO  data.scripts.world.TestScript  - Loading data for animal: dog
2112995 [Thread-4] INFO  data.scripts.world.TestScript  - Column count: 4
2112996 [Thread-4] INFO  data.scripts.world.TestScript  - Value of 'legs': 4
2112996 [Thread-4] INFO  data.scripts.world.TestScript  - Value of 'size': medium
2112996 [Thread-4] INFO  data.scripts.world.TestScript  - Loading data for animal: cat
2112996 [Thread-4] INFO  data.scripts.world.TestScript  - Column count: 4
2112996 [Thread-4] INFO  data.scripts.world.TestScript  - Value of 'legs': 4
2112997 [Thread-4] INFO  data.scripts.world.TestScript  - Value of 'size': small
2112997 [Thread-4] INFO  data.scripts.world.TestScript  - Loading data for animal: spider
2112997 [Thread-4] INFO  data.scripts.world.TestScript  - Column count: 4
2112997 [Thread-4] INFO  data.scripts.world.TestScript  - Value of 'legs': 8
2112997 [Thread-4] INFO  data.scripts.world.TestScript  - Value of 'size': small
2112997 [Thread-4] INFO  data.scripts.world.TestScript  - Loading data for animal: t-rex
2112997 [Thread-4] INFO  data.scripts.world.TestScript  - Column count: 4
2112997 [Thread-4] INFO  data.scripts.world.TestScript  - Value of 'legs': 2
2112998 [Thread-4] INFO  data.scripts.world.TestScript  - Value of 'size': large

190001 [Thread-4] INFO  org.lazywizard.console.Console  - > runcode data.scripts.world.TestScript.execute()
190707 [Thread-4] INFO  com.fs.starfarer.loading.LoadingUtils  - Loading CSV data from [DIRECTORY: D:\Program Files\Fractal Softworks\Starsector\starsector-core\..\mods\testmod]
190708 [Thread-4] INFO  com.fs.starfarer.loading.LoadingUtils  - Loading CSV data from [DIRECTORY: D:\Program Files\Fractal Softworks\Starsector\starsector-core\..\mods\testmod2]
190708 [Thread-4] INFO  data.scripts.world.TestScript  - Loading data for animal: spider
190708 [Thread-4] INFO  data.scripts.world.TestScript  - Column count: 4
190709 [Thread-4] INFO  data.scripts.world.TestScript  - Value of 'legs': 8
190709 [Thread-4] INFO  data.scripts.world.TestScript  - Value of 'size': small
190709 [Thread-4] INFO  data.scripts.world.TestScript  - Loading data for animal: t-rex
190709 [Thread-4] INFO  data.scripts.world.TestScript  - Column count: 4
190710 [Thread-4] INFO  data.scripts.world.TestScript  - Value of 'legs': 2
190710 [Thread-4] INFO  data.scripts.world.TestScript  - Value of 'size': large
190710 [Thread-4] INFO  data.scripts.world.TestScript  - Loading data for animal: dog
190710 [Thread-4] INFO  data.scripts.world.TestScript  - Column count: 3
190710 [Thread-4] INFO  data.scripts.world.TestScript  - Value of 'eggs': no
190710 [Thread-4] INFO  data.scripts.world.TestScript  - Loading data for animal: cat
190710 [Thread-4] INFO  data.scripts.world.TestScript  - Column count: 3
190710 [Thread-4] INFO  data.scripts.world.TestScript  - Value of 'eggs': no
190710 [Thread-4] INFO  data.scripts.world.TestScript  - Loading data for animal: parrot
190710 [Thread-4] INFO  data.scripts.world.TestScript  - Column count: 3
190711 [Thread-4] INFO  data.scripts.world.TestScript  - Value of 'eggs': yes
Cat and dog no longer have their legs or size values.
[close]

Minimal mods for testing
(Paste runcode data.scripts.world.TestScript.execute() into Console Commands window to execute the script)
« Last Edit: July 22, 2020, 06:11:49 AM by Histidine »
Logged

Alex

  • Administrator
  • Admiral
  • *****
  • Posts: 23986
    • View Profile
Re: [0.8.1a] Merging CSVs with different columns loses data
« Reply #1 on: May 13, 2018, 10:04:16 AM »

Aha - this is actually working like it's supposed to. There's no "merging" for CSVs, there's simply row replacement based on the id columns.
Logged

Histidine

  • Admiral
  • *****
  • Posts: 4661
    • View Profile
    • GitHub profile
Re: [0.8.1a] Merging CSVs with different columns loses data
« Reply #2 on: May 14, 2018, 05:05:38 AM »

Ah.
Well, can it be a feature request than? Or at least write a javadoc so we know "getMergedSpreadsheetDataForMod" does not actually merge.

Can mods merge arbitrary .jsons in 0.8.1, or is that only for vanilla files?
Logged

Alex

  • Administrator
  • Admiral
  • *****
  • Posts: 23986
    • View Profile
Re: [0.8.1a] Merging CSVs with different columns loses data
« Reply #3 on: May 14, 2018, 10:24:49 AM »

Hmm. I mean, it does merge, it just merges spreadsheets, not row contents. Clarified that in the comments.

I think the way you're expecting it to work would restrict a mod to a specific set of column changes, where when you're merging json, you'd really want it do be on a per-item basis.

E.G. a mod couldn't 1) add an "eggs" column but also 2) change any other data for an existing animal. Unless an empty cell didn't count as that row having that column but that would mean a mod couldn't blank out a cell. It just gets very hacky. If you're looking to do that sort of thing, it might be better to have a separate file and handle the merging yourself based on the specific requirement. Sort of like the base game has custom entities but also salvage data keyed off the same id.


Can mods merge arbitrary .jsons in 0.8.1, or is that only for vanilla files?

Can't now, but there's no real reason for it, just never got around to exposing the method. Added to SettingsAPI:
JSONObject getMergedJSONForMod(String path, String masterMod) throws IOException, JSONException;

Logged

Histidine

  • Admiral
  • *****
  • Posts: 4661
    • View Profile
    • GitHub profile
Re: [0.8.1a] Merging CSVs with different columns loses data
« Reply #4 on: September 22, 2018, 07:09:51 PM »

Question on the new JSON merge:

Spoiler
If I have two jsons:

Code: json
{
  "foo":[
    ["one", "two", "three"]
  ]
}
Code: json
{
  "foo":[
    ["aa", "bb", "cc"]
  ]
}

will foo end up as:
Code: json
  "foo":[
    ["one", "two", "three"],
    ["aa", "bb", "cc"]
  ]
or
Code: json
  "foo":[
    ["one", "two", "three", "aa", "bb", "cc"]
  ]
or
Code: json
  "foo":[
    ["aa", "bb", "cc"]
  ]
?

(The one I need is #1, but I can see use cases for all three. Hurm, perhaps I ought to just use a CSV and give each entry that would go into foo a unique ID.)
[close]
Logged

Alex

  • Administrator
  • Admiral
  • *****
  • Posts: 23986
    • View Profile
Re: [0.8.1a] Merging CSVs with different columns loses data
« Reply #5 on: September 22, 2018, 09:48:14 PM »

Offhand, I think it's #1, but since I'm pretty sure this is not a case that's come up in vanilla, there could be bugs :)
Logged

Histidine

  • Admiral
  • *****
  • Posts: 4661
    • View Profile
    • GitHub profile
Re: [0.8.1a] Merging CSVs with different columns loses data
« Reply #6 on: July 22, 2020, 06:11:59 AM »

Bumping to say that I think there is a case for implementation of CSV row merging. This would address some concerns with mods trying to make minimally invasive changes without potential conflicts with vanilla/other mods.

Two cases that arose in this regard:
- Upgraded Rotary Weapons changes some vanilla weapons to add spinup/down times (so the animations work properly). However, the current system requires it to replace the entire CSV row, including gameplay-related parameters such as damage. This caused a (brief, minor) issue following the release of Starsector 0.9.1a, when the mod was overriding the Assault Chaingun buff.

- I recently wanted to change Nexerelin to prevent Guardian and derelict mothership blueprints from dropping during raids, but didn't want to replace the whole ship_data row and thereby create incompatibility with any mods that would change the Guardian (especially seeing as combat stuff is generally none of Nex's business). I ended up doing a codeside hack in my MarketCMD implementation.
Logged

Alex

  • Administrator
  • Admiral
  • *****
  • Posts: 23986
    • View Profile
Re: [0.8.1a] Merging CSVs with different columns loses data
« Reply #7 on: July 22, 2020, 10:09:14 AM »

Hmm, right. I get where you're coming from, but making rows merge per-column is still troublesome.

However: the workaround/intended way to do this is to make these kinds of targeted changes in code in onApplicationLoad(). In your case, it seems like you should be able to accomplish this using ShipHullSpecAPI.addTag(), getHints().add(), etc.

If the API methods for the relevant *Spec classes don't have the right setter methods exposed, I'd be happy to take a look at that!
Logged

Histidine

  • Admiral
  • *****
  • Posts: 4661
    • View Profile
    • GitHub profile
Re: [0.8.1a] Merging CSVs with different columns loses data
« Reply #8 on: July 27, 2020, 01:35:58 AM »

That sounds like a fair way to do it, thanks.

(Forgot to say regarding the empty cell merging thing: my idea was that if a merging CSV doesn't contain that column, it doesn't change the final merged value for that cell, whereas a defined empty cell in one of the CSVs blanks it. Or use a specific string value to clear it, like we're getting for JSON array merge.)
Logged

Alex

  • Administrator
  • Admiral
  • *****
  • Posts: 23986
    • View Profile
Re: [0.8.1a] Merging CSVs with different columns loses data
« Reply #9 on: July 27, 2020, 09:04:57 PM »

Ah, yeah, that'd make sense! I'll keep it in mind in case I end up messing with related code at some point.
Logged