-
Notifications
You must be signed in to change notification settings - Fork 37
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
Output JSON reports - part 1 #150
Conversation
// ie "Alioth Plains (1, 2) in Alioth contains Alioth City [City]." | ||
// This will get broken down and become more json-y when I flesh this code out. | ||
j["name"] = Print(pRegions).const_str(); | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is here solely so that the output shows that it is iterating the regions.. This is just a temp leaf.
"won game" | ||
"game over", | ||
}; | ||
const string *QuitStrs = qs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed these so I could output a textual string for the various quit states in the faction json.
if (citems[pl][i] > citems[myfaction][i]) place++; | ||
if (max < citems[pl][i]) max = citems[pl][i]; | ||
total += citems[pl][i]; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you had already iterated all items for all units for all factions while building the citems array, there was no need to do so again here, so I just rewrote this to use the data you'd already collected :p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, it now returns it as a vector so I can output it in textual and json formats as needed.
battles.DeleteAll(); | ||
} | ||
|
||
void Faction::write_json_gm_report(json& j, Game *game) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the gm reports were taking up a lot of space and making the report function hard to read, so I pulled all the gm stuff out into it's own pair of functions. I also just collect all the data up front and output it. This made the code cleaner imho.
@@ -559,34 +790,27 @@ void Faction::WriteReport(ostream& f, Game *pGame, int ** citems) | |||
f << '\n'; | |||
} | |||
|
|||
if (shows.Num()) { | |||
if (!shows.empty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I converted shows/objectshows/itemsshows into std::vector<> rather than Alists.. Slowly but surely, AList and AString will die too!
Because these changes are going to be somewhat far-reaching, I am going to do these in smaller chunks. This chunk will contain - a new game config which defaults to not outputting json reports. - the inclusion of the nlohmann/json.hpp single-file json parser/manipulation library. - Output the basic faction info (meaning the fields on the faction itself, but NOT deeper structures like regions/objects/units) - An example of some small nested structures (skill shows/faction stats) to give examples of how this stuff will work. - A renaming of 2 rules helper functions from link/link_ref to url/achor in order to avoid a conflict with the link() function from unistd.h which gets pulled in via the nlohmann/json.hpp standard library includes, and which caused the rules to break. This is JUST a renaming and no functionality change, so you can ignore all the changes in genrules.cpp as part of this. - Removing some duplicated skill data which was being output if you were shown a skill description which allowed you create an object which in turn allowed you to use a skill (create staff of healing shows the staff of healing, which then adds a skill show for magic healing). This wouldn't affect a player since a faction tracks whether or not a skill has been shown permanently and never shows it again, but when generating the turn 1 report for the gamemaster, if those items were enabled, 4 skills would end up shown twice. This also accounts for the change to the snapshot turn_0 report.1 file. Those changes are expected.
8b99d40
to
690a2ce
Compare
// Rather than making all of the functions & variables public, we can allow this special class to access them. | ||
// This class won't be defined except in the unit test code, though it could be used in other places if there | ||
// were a good reason to do so. | ||
friend class UnitTestHelper; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was the hack that I mentioned which allows me to do deeper unit testing where things need to get at internals of game but couldn't. I honestly am a big believer in 'only make public the things that should be public', so will probably over time be privatizing things in other classes/structs that are only used internally, and will likely grant other UnitTestHelper friend status to those other classes if needed.
@@ -221,12 +221,6 @@ int StudyRateAdjustment(int days, int exp) | |||
return rate; | |||
} | |||
|
|||
ShowSkill::ShowSkill(int s, int l) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made this into a struct with static initialization because it was a lot clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These 4 skills with just a couple ranks were caused by the enabled skills and objects such as create staff of healing, and the staff of healing. When you learned create staff of healing, it would also add the staff of healing item if your faction hadn't already discovered it, and adding that would add the magical healing skill if your faction hadn't discovered it.
Due to the way that gm reports work (where it just produces the skill show without 'discovering' that skill or item for the faction), these ended up duplicated. I could have changed the GM report to discover those items, but discovered items are stored in the game.out file for each faction and thus would have changed every snapshot turns game.out file rather than just changing the turn 0 report.1 file. Since this only affected GM reports for the first turn this seemed reasonable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of utility shell scripts to allow updating the snapshots of the games & rules if something changes that you expect to change (such as us changing output format or something or fixing a typo). Those would initially fail the test and then you can update all snapshots if the change was intended.
@@ -36,8 +36,31 @@ | |||
#include "gamedata.h" | |||
|
|||
/// Run the initial setup for a faction | |||
/// which for a unit test is to do nothing | |||
int Game::SetupFaction( Faction *pFac ) { return 1; } | |||
/// For unit tests, factions get 1 leader with combat 3. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to unit test json reports, it still needs a valid game object correctly set up, so I construct a unit test world which has exactly 4 hexes and 1 city. The test will also have guards and wandering monsters, so those 2 factions are taken up as well.
@@ -70,6 +78,7 @@ void ARegionList::MakeRegions(int level, int xSize, int ySize) | |||
|
|||
Add(reg); | |||
arr->SetRegion(x, y, reg); | |||
Adot(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to make it easy to see how many regions got created since there are only 4 and it made testing the cout capturing have some data.
@@ -168,7 +168,7 @@ static GameDefs g = { | |||
0, // UNDERWORLD_LEVELS | |||
0, // UNDERDEEP_LEVELS | |||
0, // ABYSS_LEVEL | |||
100, // TOWN_PROBABILITY | |||
0, // TOWN_PROBABILITY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want it to spawn anything other than the 1 city I created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is the single-include json handling library. Like the unit test framework (ut), it requires no external libraries or links so it will just work.
Niels Lohmann's JSON library is finally here. It is an excellent addition to the codebase. This will allow many future changes. I.e. game database files can be changed to the JSON format. It would make them less fragile and readable from the outside for different GM needs. |
That is almost certainly in my plans for the future :) |
Because these changes are going to be somewhat far-reaching, I am going
to do these in smaller chunks. This chunk will contain