Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Many mods have json errors that need to be cleared up #36171

Closed
51 tasks done
jbytheway opened this issue Dec 17, 2019 · 6 comments
Closed
51 tasks done

Many mods have json errors that need to be cleared up #36171

jbytheway opened this issue Dec 17, 2019 · 6 comments
Labels
<Bug> This needs to be fixed [JSON] Changes (can be) made in JSON Mods Issues related to mods or modding

Comments

@jbytheway
Copy link
Contributor

jbytheway commented Dec 17, 2019

Describe the bug

Recent work on various fronts has substantially improved the validation of json game data in the CI testing. However, the errors were only checked for the core game data (and a couple of mods).

Now #35995 has added a test which is intended to load the data for all mods, so that they can all be checked in CI and errors don't creep into the mods.

However, many mods still have errors, so currently there is a blacklist of mods which are not checked.

The hope is that each mod can now be fixed to load cleanly, and this work can be done piecemeal. As each mod is fixed it can be removed from the blacklist. Ultimately, we want the blacklist to be unnecessary.

If a mod you care about is blacklisted, then I encourage you to try fixing it and make a suitable PR.

To see the errors locally, run the tests (tests/cata_test) with the --mods= option, listing the id of the mod in question. For example:

./tests/cata_test --mods=magiclysm '~*'

The '~* parameter causes it to not actually run any of the tests; for the purposes of this work running the tests is probably not necessary, because the errors arise during the loading of game data, before any tests run.

Most of the problems are simple misunderstandings of the json format. To get some inspiration on how to resolve errors, see the previous PRs that fixed the core game data #34455 and Magiclysm #34525.

Additional context

Here is a list of the currently blacklisted mods that need to be fixed:

  • aftershock
  • alt_map_key
  • Animatronics
  • Battery_Overhaul_Legacy_Mode
  • blazemod
  • cbm_slots
  • classic_zombies
  • crazy_cataclysm
  • crt_expansion
  • desertpack
  • DinoMod
  • ew_pack
  • FIC_Weapons
  • FujiStruct
  • generic_guns
  • Graphical_Overmap
  • growable-pots
  • Heavy miners
  • hydroponics
  • magiclysm
  • manualbionicinstall
  • mapgen_demo
  • Medieval_Stuff
  • MMA
  • modular_turrets
  • more_locations
  • More_Survival_Tools
  • mutant_npcs
  • my_sweet_cataclysm
  • national_guard_camp
  • No_Anthills
  • No_Bees
  • no_faults
  • no_filthy_clothing
  • No_Fungi
  • no_medieval_items
  • no_npc_food
  • No_Rail_Stations
  • No_Triffids
  • novitamins
  • realguns
  • safeautodoc
  • Salvaged_Robots
  • sees_player_hitbutton
  • sees_player_retro
  • sleepdeprivation
  • speedydex
  • stats_through_kills
  • Tanks
  • Tolerate_This
  • Urban_Development
@Maleclypse
Copy link
Member

I use Github Desktop and Atom for JSON editing. Is there another tool I need to get to run the tests as you describe above?

@jbytheway
Copy link
Contributor Author

To make it easy for yourself, you need to compile the code yourself. In theory we could distribute the test executable alongside the game to allow you to test without compiling, but currently that's not done. This is a rare niche circumstance in which it is useful, because generally you wouldn't need to run the tests unless you were editing the code.

If you are willing to be more patient, however, you can rely on Travis to run the tests for you. Open a PR where you remove a mod from the blacklist (mark it WIP) and see what errors Travis reports for you to fix.

@ZhilkinSerg
Copy link
Contributor

Blacklist as of today:

aftershock
blazemod
crt_expansion
generic_guns
Graphical_Overmap
mapgen_demo
more_locations
no_medieval_items
sees_player_hitbutton
sees_player_retro

@jbytheway
Copy link
Contributor Author

We're now down to just two mods blacklisted:

more_locations
Graphical_Overmap_More_Locations

(and the latter only because it depends on the former).

more_locations is in the process of being slowly transitioned to core content. It probably won't be unblacklisted until that's done.

@hexagonrecursion
Copy link
Contributor

mod_test_blacklist was removed by #46337

@jbytheway
Copy link
Contributor Author

Yes, this can be closed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bug> This needs to be fixed [JSON] Changes (can be) made in JSON Mods Issues related to mods or modding
Projects
None yet
Development

No branches or pull requests

6 participants