Fractal Softworks Forum

Please login or register.

Login with username, password and session length

Author Topic: illegal json in some data files. (trailing commas in object definitions)  (Read 2586 times)

TJJ

  • Admiral
  • *****
  • Posts: 1905
    • View Profile

Not sure if this should be considered a bug or not.
It obviously doesn't cause any trouble for the game itself, but may cause problems for any hypothetical 3rd party modding tools that are being developed  :P

I only noticed it because Google's Gson parser is rather intolerant of syntax errors (makes it explode even when operating in lenient mode!).
I guess whatever parser Starfarer uses is rather more forgiving.

The list of affected files is rather large, but the erroneous commas are (as far as I can tell) exclusively found at the end of the files. (perhaps a bug in whatever tool chain is used for generating the data files?)

The affected json files are:
Spoiler
Quote
onslaught.ship
ammofeed.system
burndrive.system
displacer.system
drone_pd.system
drone_pd_x2.system
drone_sensor.system
drone_terminator.system
emp.system
fastmissileracks.system
flarelauncher.system
flarelauncher_active.system
flarelauncher_fighter.system
fortressshield.system
highenergyfocus.system
inferniuminjector.system
maneuveringjets.system
phasecloak.system
phaseteleporter.system
skimmer_drone.system
flare_fighter.proj
flare_seeker.proj
flare_standard.proj
dualflak_shot.proj
flak_shot.proj
fragbomb_proj.proj
phase_charge.proj
plasma_shot.proj
sabot_warhead.proj
shredder_shot.proj
tpc_shot.proj
flarelauncher1.wpn
flarelauncher2.wpn
flarelauncher3.wpn
amblaster.wpn
annihilator.wpn
annihilatorpod.wpn
arbalest.wpn
atropos.wpn
atropos_single.wpn
autopulse.wpn
bomb.wpn
chaingun.wpn
clusterbomb.wpn
cyclone.wpn
dualflak.wpn
flak.wpn
fragbomb.wpn
gauss.wpn
gravitonbeam.wpn
guardian.wpn
harpoon.wpn
harpoonpod.wpn
harpoon_single.wpn
heatseeker.wpn
heavyac.wpn
heavyblaster.wpn
heavyburst.wpn
heavymauler.wpn
heavymg.wpn
heavyneedler.wpn
hellbore.wpn
hephag.wpn
hil.wpn
hurricane.wpn
hveldriver.wpn
interdictorbeam.wpn
ioncannon.wpn
irpulse.wpn
lightac.wpn
lightag.wpn
lightdualac.wpn
lightdualmg.wpn
lightmg.wpn
lightmortar.wpn
lightneedler.wpn
lrpdlaser.wpn
mark9.wpn
miningblaster.wpn
mininglaser.wpn
mjolnir.wpn
multineedler.wpn
pdburst.wpn
pdlaser.wpn
phasebeam.wpn
phasecl.wpn
pilum.wpn
plasma.wpn
pulselaser.wpn
railgun.wpn
reaper.wpn
sabot.wpn
sabotpod.wpn
sabot_single.wpn
salamanderpod.wpn
shredder.wpn
swarmer.wpn
tachyonlance.wpn
taclaser.wpn
tpc.wpn
typhoon.wpn
vulcan.wpn
enforcer_Support.variant
tempest_Balanced.variant
drone_assault.variant
drone_pd.variant
drone_pd_midline.variant
drone_sensor.variant
drone_terminator.variant
broadsword_Fighter.variant
dagger_Bomber.variant
gladius_Fighter.variant
longbow_Support.variant
mining_drone_Standard.variant
piranha_Bomber.variant
talon_Interceptor.variant
thunder_Fighter.variant
trident_Bomber.variant
warthog_Fighter.variant
wasp_Interceptor.variant
xyphos_Fighter.variant
hegemony.faction
independent.faction
neutral.faction
pirates.faction
player.faction
tritachyon.faction
[close]
« Last Edit: September 13, 2012, 12:35:23 PM by TJJ »
Logged

Alex

  • Administrator
  • Admiral
  • *****
  • Posts: 24114
    • View Profile

Thanks for the report. Not a bug, though - Starfarer uses a slightly modified form of JSON (the commas you mentioned, but also, you can use # for comments). Shouldn't cause any serious problems because it'll also parse anything that's valid, standard JSON - but yeah, any tool reading the files will need to do a bit of pre-processing.

Btw: the reason those extra commas are in there is for ease of copy-and-pasting (don't have to worry whether something is the last line or not). I wouldn't have actually added something like that, but the JSON parser I'm using was already ok with it. Also nice if you want to shuffle the order of things in an array, etc.
« Last Edit: September 13, 2012, 12:00:40 PM by Alex »
Logged

TJJ

  • Admiral
  • *****
  • Posts: 1905
    • View Profile

Btw: the reason those extra commas are in there is for ease of copy-and-pasting (don't have to worry whether something is the last line or not). I wouldn't have actually added something like that, but the JSON parser I'm using was already ok with it. Also nice if you want to shuffle the order of things in an array, etc.

Totally agree.
I was rather surprised to learn the Json spec. didn't support trailing commas, or comments. Talk about a usability oversight...

Fortunately Gson already allows # and // comments (when run in lenient mode), and modifying it to allow trailing commas after objects wasn't too difficult.
Logged

Alex

  • Administrator
  • Admiral
  • *****
  • Posts: 24114
    • View Profile
Re: illegal json in some data files. (trailing commas in object definitions)
« Reply #3 on: September 13, 2012, 01:34:11 PM »

Yeah... I can see an argument for keeping the spec clean, as it's intended for data exchange (I think?) rather than storage. Still, comments would have been nice. I'm using this library to parse it (with preprocessing to strip out comments), btw - very light-weight and non-verbose to use.

Gson looks good at first glance, too. Reading directly into an object is nice, almost tempting enough to make me try it out. Might run into some unanticipated problems, though - you know how it is with things that look really good at first glance :)
Logged

Ghoti

  • Captain
  • ****
  • Posts: 283
    • View Profile
Re: illegal json in some data files. (trailing commas in object definitions)
« Reply #4 on: September 13, 2012, 01:43:55 PM »

arn't json files supposed to be valid javascript? I don't think it's valid javascript with trailing commas.
Logged

TJJ

  • Admiral
  • *****
  • Posts: 1905
    • View Profile
Re: illegal json in some data files. (trailing commas in object definitions)
« Reply #5 on: September 13, 2012, 01:59:03 PM »

It's really neat the way Gson transparently handles primitive types, enums, Collections, Maps, generics, nested objects, in fact pretty much everything I've encountered so far!
It does it all through direct field access, so you don't have to compromise encapsulation with public setters for all your fields. (à la java beans)

Only one usability problem I've encountered so far is if the json contains an object field that has no corresponding equivalent in the Java class; it silently ignores it.
Ideally I'd want to flag this kind of parsing error to the dev/modder, as they've probably misspelled a property name in the json file.
It should be trivial to add in though.

Here's a little example; obviously there's no logic for manipulating the data in a meaningful way yet - have to design the UI first  ;D
The interesting bit is.... the complete lack of anything interesting!
1 line of code to load & parse an entire json file into its corresponding java class heirarchy; awesome.

Spoiler
Code
package uk.tjj.staredit.data;

import java.io.File;
import java.io.FileReader;
import java.util.ArrayList;
import java.util.TreeMap;

import com.google.gson.Gson;

public class Ship {

private ArrayList<Integer> bounds;

private TreeMap<String, String> builtInWeapons;

private int[] center;

private int collisionRadius;

private ArrayList<EngineSlot> engineSlots;

private int height;

private String hullId;
private String hullName;

private HullClass hullSize;

public static enum HullClass {
CAPITAL_SHIP, CRUISER, DESTROYER, FIGHTER, FRIGATE,
}

private int[] shieldCenter;
private int shieldRadius;
private String spriteName;
private Style style;

public static enum Style {
HIGH_TECH, LOW_TECH, MIDLINE
}

private int viewOffset;

private ArrayList<WeaponSlot> weaponSlots;

private int width;

public static void main(String[] args) throws Exception {
File testFile = new File(
"C:\\Program Files (x86)\\Fractal Softworks\\Starfarer\\starfarer-core\\data\\hulls\\",
"onslaught.ship");

Gson gson = new Gson();

Ship testShip = gson.fromJson(new FileReader(testFile), Ship.class);

testShip.toString();
}
}
[close]
« Last Edit: September 13, 2012, 04:08:48 PM by TJJ »
Logged

Trylobot

  • Global Moderator
  • Admiral
  • *****
  • Posts: 1170
    • View Profile
    • Github profile
Re: illegal json in some data files. (trailing commas in object definitions)
« Reply #6 on: September 13, 2012, 03:31:06 PM »

I'm rather proud of the lib I wrote to handle Starfarer's JSON format (though it is bound to a language that very few people use) since I used the available data files as the dictionary of what is valid, and made a hefty number of (apparently) correct assumptions about how certain things would be handled.

https://github.com/Trylobot/sf-ship-ed/blob/master/src/rjson.bmx

I also approve of the modifications to the spec.. especially comments.
Logged

TJJ

  • Admiral
  • *****
  • Posts: 1905
    • View Profile
Re: illegal json in some data files. (trailing commas in object definitions)
« Reply #7 on: September 13, 2012, 04:04:01 PM »

I used the available data files as the dictionary of what is valid

Wow, what a coincidence; I just finished *bodging something together to do precisely that!

I was contemplating using this scraped data to auto-generate the source files for the data storage classes, so any future changes to the json format would be automatically accounted for. (self-maintaining code, w00h00!)
Though in the long term I think the complexity and inflexibility it'd introduce would cost more time than it'd save, so I'll probably not bother and just maintain the code myself whenever the json data changes/expands.

*hideous code I'd be too embarrassed to ever show!
The output is useful as reference material though.
Spoiler
Code
---------------.ship---------------
{
      bounds=[-1, -10, -100, -101, -102, -103, -104, -105, -106, -11, -110, -112, -113, -114, -115, -116, -117, -118, -12, -120, -121, -122, -123, -124, -125, -126, -127, -13, -130, -132, -134, -136, -139, -14, -140, -142, -143, -144, -146, -15, -156, -157, -158, -16, -163, -167, -168, -169, -17, -171, -172, -173, -176, -177, -178, -18, -180, -181, -185, -19, -2, -20, -21, -22, -23, -24, -25, -26, -27, -28, -29, -3, -30, -31, -32, -33, -34, -35, -36, -37, -38, -39, -4, -40, -41, -42, -43, -44, -45, -46, -47, -48, -49, -5, -50, -51, -52, -53, -54, -55, -56, -57, -58, -59, -6, -60, -61, -62, -63, -64, -65, -66, -67, -68, -69, -7, -70, -71, -72, -73, -74, -75, -76, -77, -78, -79, -8, -80, -81, -82, -84, -85, -86, -88, -89, -9, -90, -91, -92, -93, -94, -95, -96, -97, -98, -99, 0, 1, 10, 100, 101, 102, 103, 104, 105, 106, 107, 109, 11, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 12, 120, 121, 122, 123, 125, 126, 127, 128, 129, 13, 131, 133, 134, 136, 137, 138, 139, 14, 140, 141, 143, 144, 145, 146, 147, 149, 15, 154, 156, 157, 16, 163, 165, 168, 169, 17, 170, 173, 175, 178, 18, 180, 181, 182, 184, 186, 188, 19, 192, 194, 195, 197, 198, 199, 2, 20, 201, 203, 206, 208, 209, 21, 210, 213, 214, 215, 218, 22, 220, 221, 223, 224, 23, 230, 231, 24, 241, 248, 249, 25, 26, 264, 27, 270, 28, 29, 3, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 4, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 5, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 6, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 7, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 8, 80, 81, 83, 84, 85, 86, 87, 88, 89, 9, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99],
      builtInWeapons={
           WS 016=[tpc],
           WS 017=[tpc]},
      center=[10, 100, 105, 108, 110, 111, 115, 12, 123, 13, 14, 140, 144, 15, 155, 16, 161, 164, 17, 177, 178, 189, 19, 24, 26, 27, 28, 29, 31, 32, 34, 35, 36, 38, 39, 40, 42, 45, 46, 48, 50, 52, 54, 55, 56, 6, 60, 63, 64, 67, 68, 69, 7, 70, 71, 72, 79, 80, 81, 83, 84, 85, 86, 88, 89, 9, 91, 94, 95, 97, 99],
      collisionRadius=[105, 106, 110, 111, 112, 12, 120, 130, 135, 14, 140, 146, 15, 16, 170, 180, 182, 20, 200, 22, 220, 24, 25, 270, 275, 288, 30, 300, 330, 35, 36, 39, 58, 60, 64, 70, 75, 80, 85, 97],
      engineSlots={
           angle=[145, 160, 165, 170, 180, 190, 195, 200, 225],
           contrailSize=[128, 48, 50, 60, 64, 80],
           length=[100, 12, 16, 20, 24, 28, 32, 40, 48, 50, 55, 56, 64, 65, 72, 75, 8, 80, 96],
           location=[-10, -100, -101, -103, -104, -105, -106, -109, -11, -110, -111, -112, -114, -116, -117, -118, -119, -12, -120, -127, -13, -131, -134, -135, -139, -14, -148, -15, -16, -160, -17, -171, -172, -174, -175, -176, -177, -179, -18, -187, -189, -19, -2, -20, -21, -22, -23, -24, -25, -26, -27, -28, -29, -3, -30, -31, -32, -33, -34, -35, -36, -37, -38, -39, -4, -40, -41, -42, -43, -44, -45, -46, -48, -49, -5, -52, -53, -54, -55, -56, -58, -59, -6, -60, -61, -62, -63, -64, -65, -66, -67, -68, -69, -7, -70, -71, -72, -73, -74, -75, -76, -78, -8, -81, -82, -83, -84, -85, -9, -94, -95, -99, 0, 1, 10, 104, 109, 11, 12, 13, 139, 14, 15, 16, 17, 19, 2, 20, 21, 22, 24, 25, 26, 27, 28, 3, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 4, 40, 42, 43, 44, 46, 5, 50, 52, 53, 55, 56, 6, 61, 62, 64, 65, 67, 68, 69, 7, 70, 72, 75, 76, 78, 8, 83, 9],
           style=[HIGH_TECH, LOW_TECH, LOW_TECH_FIGHTER, MIDLINE],
           width=[12, 14, 16, 20, 24, 30, 32, 4, 5, 6, 8]},
      height=[101, 120, 13, 132, 136, 138, 14, 15, 150, 160, 164, 180, 190, 192, 204, 21, 218, 22, 222, 234, 238, 24, 260, 27, 28, 280, 33, 34, 35, 364, 38, 384, 400, 43, 440, 54, 63, 72, 80, 84, 92, 96],
      hullId=[afflictor, apogee, astral, atlas, aurora, brawler, broadsword, buffalo, buffalo2, condor, conquest, dagger, dominator, doom, dram, drone_assault, drone_pd, drone_pd_midline, drone_sensor, drone_terminator, eagle, enforcer, falcon, gemini, gladius, hammerhead, hound, hyperion, lasher, longbow, medusa, mining_drone, mule, odyssey, omen, onslaught, paragon, phaeton, piranha, shade, shuttle, shuttlepod, sunder, talon, tarsus, tempest, thunder, trident, valkyrie, venture, vigilance, warthog, wasp, wolf, xyphos],
      hullName=[Afflictor, Apogee, Assault Drone, Astral, Atlas, Aurora, Brawler, Broadsword, Buffalo, Buffalo Mk.II, Condor, Conquest, Dagger, Dominator, Doom, Dram, Eagle, Enforcer, Falcon, Gemini, Gladius, Hammerhead, Hound, Hyperion, Lasher, Longbow, Medusa, Mining Drone, Mule, Odyssey, Omen, Onslaught, PD Drone, Paragon, Phaeton, Piranha, Sensor Drone, Shade, Shuttle, Shuttle Pod, Sunder, Talon, Tarsus, Tempest, Terminator Drone, Thunder, Trident, Valkyrie, Venture, Vigilance, Warthog, Wasp, Wolf, Xyphos],
      hullSize=[CAPITAL_SHIP, CRUISER, DESTROYER, FIGHTER, FRIGATE],
      shieldCenter=[-5, -6, 0, 10, 11, 12, 14, 15, 17, 2, 20, 22, 23, 24, 25, 3, 31, 39, 6, 7, 8, 84],
      shieldRadius=[100, 106, 110, 115, 12, 120, 125, 128, 135, 14, 15, 155, 16, 160, 170, 180, 190, 20, 200, 22, 230, 24, 240, 25, 250, 270, 28, 30, 35, 53, 55, 60, 64, 70, 75, 80, 90, 96],
      spriteName=[graphics/ships/apogee_cx.png, graphics/ships/astral_cv.png, graphics/ships/atlas_af.png, graphics/ships/aurora_ca.png, graphics/ships/brawler.png, graphics/ships/broadsword_hftr.png, graphics/ships/buffalo_af.png, graphics/ships/buffalo_dd.png, graphics/ships/condor_dd.png, graphics/ships/conquest_bc.png, graphics/ships/dagger_trp.png, graphics/ships/dominator.png, graphics/ships/drones/drone_com.png, graphics/ships/drones/drone_pd.png, graphics/ships/drones/drone_phase.png, graphics/ships/drones/drone_strike.png, graphics/ships/eagle_ca.png, graphics/ships/enforcer_dd.png, graphics/ships/falcon_cl.png, graphics/ships/fighter_assault_midline.png, graphics/ships/fighter_heavy_midline.png, graphics/ships/gemini.png, graphics/ships/hammerhead_dd.png, graphics/ships/hound_ff.png, graphics/ships/hyperion_ff.png, graphics/ships/lasher_ff.png, graphics/ships/longbow_intg.png, graphics/ships/medusa.png, graphics/ships/mining_drone.png, graphics/ships/mule_dd.png, graphics/ships/odyssey.png, graphics/ships/omen_ff.png, graphics/ships/onslaught_bb.png, graphics/ships/paragon.png, graphics/ships/phase/phase_assault_ff.png, graphics/ships/phase/phase_ca.png, graphics/ships/phase/phase_strike_ff.png, graphics/ships/piranha_bmb.png, graphics/ships/shuttle.png, graphics/ships/shuttle_modern.png, graphics/ships/sunder_ca.png, graphics/ships/talon_ftr.png, graphics/ships/tanker_medium.png, graphics/ships/tanker_small.png, graphics/ships/tarsus.png, graphics/ships/tempest.png, graphics/ships/thunder.png, graphics/ships/trident.png, graphics/ships/valkyrie_ap.png, graphics/ships/venture.png, graphics/ships/vigilance.png, graphics/ships/wasp_ftr.png, graphics/ships/wolf_ff.png, graphics/ships/xyphos.png],
      style=[HIGH_TECH, LOW_TECH, MIDLINE],
      viewOffset=[0, 100, 150, 200, 50],
      weaponSlots={
           angle=[-20, -30, -40, -45, -50, -60, -65, -70, -75, -80, -90, 0, 100, 105, 110, 120, 135, 140, 145, 150, 155, 160, 170, 180, 190, 20, 200, 205, 210, 215, 220, 225, 240, 250, 255, 260, 30, 45, 50, 60, 65, 70, 75, 80, 90],
           arc=[0, 10, 120, 125, 135, 14, 140, 15, 150, 159, 160, 170, 180, 190, 200, 210, 215, 220, 225, 235, 250, 270, 275, 290, 30, 315, 330, 360, 5, 60, 90],
           id=[WS 001, WS 002, WS 003, WS 004, WS 005, WS 006, WS 007, WS 008, WS 009, WS 010, WS 011, WS 012, WS 013, WS 014, WS 015, WS 016, WS 017, WS 018, WS 019, WS 020, WS 021, WS 022, WS 023, WS 024],
           locations=[-1, -10, -100, -103, -104, -109, -11, -111, -112, -114, -12, -121, -122, -126, -13, -136, -14, -143, -145, -15, -150, -16, -17, -18, -19, -2, -20, -21, -22, -23, -24, -25, -26, -27, -28, -29, -3, -30, -31, -32, -33, -34, -35, -38, -39, -4, -40, -41, -42, -43, -44, -45, -46, -47, -48, -49, -5, -50, -51, -52, -53, -54, -55, -56, -57, -58, -59, -6, -60, -61, -62, -65, -66, -68, -69, -7, -70, -71, -72, -73, -76, -78, -8, -80, -81, -83, -86, -87, -88, -89, -9, -90, -92, -95, -99, 0, 1, 10, 100, 102, 104, 106, 107, 108, 109, 11, 110, 112, 113, 114, 115, 118, 119, 12, 121, 123, 125, 126, 127, 128, 129, 13, 131, 132, 133, 134, 136, 138, 139, 14, 140, 142, 146, 147, 148, 149, 15, 157, 158, 16, 160, 161, 163, 165, 166, 17, 171, 18, 184, 188, 19, 198, 199, 2, 20, 201, 21, 22, 23, 24, 241, 25, 26, 265, 27, 28, 29, 3, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 4, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 5, 51, 52, 53, 55, 56, 57, 59, 6, 60, 61, 62, 63, 65, 66, 67, 68, 69, 7, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 8, 80, 81, 82, 83, 85, 86, 87, 88, 89, 9, 91, 92, 94, 96, 99],
           mount=[HARDPOINT, HIDDEN, TURRET],
           position=[-124, -22, -24, -50, -65, -75, 0, 121, 131, 50, 51, 65, 80],
           size=[LARGE, MEDIUM, SMALL],
           type=[BALLISTIC, BUILT_IN, ENERGY, LAUNCH_BAY, MISSILE, SYSTEM, UNIVERSAL]},
      width=[100, 105, 108, 110, 112, 120, 128, 134, 136, 14, 140, 162, 168, 18, 190, 194, 20, 200, 220, 24, 26, 288, 30, 32, 320, 330, 34, 38, 52, 58, 62, 63, 64, 66, 68, 72, 76, 80, 90]}
--------------------------------------
[close]
« Last Edit: September 13, 2012, 04:09:08 PM by TJJ »
Logged