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.

Sign up for GitHub

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

Fix Issue #997 #998

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
+40 −67
Open

Fix Issue #997 #998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4ec7b97
unit_csv.cpp: Fix issue #997
stephengtuggy Jan 17, 2025
12c7b17
unit_json_factory.cpp: When reading, the root of the file JSON is an …
stephengtuggy Jan 17, 2025
07579b4
unit_json_factory.cpp: More on reading and parsing
stephengtuggy Jan 17, 2025
2625c01
Peer feedback: omit several functions and constructors that aren't ne…
stephengtuggy Jan 17, 2025
35d07a1
unit_json_factory.cpp: Fix error message as per peer review
stephengtuggy Jan 18, 2025
db4d8c1
unit_csv.cpp: Put an array at the root of the JSON file that gets sav…
stephengtuggy Jan 18, 2025
21a399f
Merge branch 'master' into fix/issue_997
stephengtuggy Jan 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions engine/src/cmd/csv.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*
* Copyright (C) Daniel Horn
* Copyright (C) 2020 pyramid3d, Stephen G. Tuggy, and other Vega Strike contributors
* Copyright (C) 2021-2022 Stephen G. Tuggy
* Copyright (C) 2021-2025 Stephen G. Tuggy
*
* https://github.com/vegastrike/Vega-Strike-Engine-Source
*
Expand Down Expand Up @@ -291,11 +291,6 @@ CSVRow::CSVRow(CSVTable *parent, const string &key) {
iter = parent->rows[key] * parent->key.size();
}

CSVRow::CSVRow(CSVTable *parent, unsigned int i) {
this->parent = parent;
iter = i * parent->key.size();
}

const string &CSVRow::operator[](const string &col) const {
static string empty_string;
vsUMap<string, int>::iterator i = parent->columns.find(col);
Expand Down
1 change: 0 additions & 1 deletion engine/src/cmd/csv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ class CSVRow {
}

CSVRow(CSVTable *parent, const std::string &key);
CSVRow(CSVTable *parent, unsigned int which);

CSVRow() {
parent = NULL;
Expand Down
58 changes: 10 additions & 48 deletions engine/src/cmd/unit_csv.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,38 +65,14 @@ extern void pushMesh(std::vector<Mesh *> &mesh,
void addShieldMesh(Unit::XML *xml, const char *filename, const float scale, int faction, class Flightgroup *fg);
void addRapidMesh(Unit::XML *xml, const char *filename, const float scale, int faction, class Flightgroup *fg);

// TODO: This is a terrible kludge. Replace with boost::json
std::string MapToJson(std::map<std::string, std::string> unit) {
std::string json_string = "[\n\t{\n";

int len = unit.size();
int i = 0;

for (auto const& pair : unit) {
boost::format new_line;
if(i < len-1) {
new_line = boost::format("\t\t\"%1%\": \"%2%\",\n") % pair.first % pair.second;
} else {
new_line = boost::format("\t\t\"%1%\": \"%2%\"\n") % pair.first % pair.second;
}

i++;
json_string += new_line.str();
}

json_string += "\t}\n]\n";

return json_string;
}

void AddMeshes(std::vector<Mesh *> &xmeshes,
float &randomstartframe,
float &randomstartseconds,
float unitscale,
const std::string &meshes,
int faction,
Flightgroup *fg,
vector<unsigned int> *counts) {
float &randomstartframe,
float &randomstartseconds,
float unitscale,
const std::string &meshes,
int faction,
Flightgroup *fg,
vector<unsigned int> *counts) {
string::size_type where, when, wheresf, wherest, ofs = 0;

// Clear counts vector
Expand Down Expand Up @@ -1125,22 +1101,6 @@ void Unit::LoadRow(std::string unit_identifier, string modification, bool saved_
this->num_chunks = UnitCSVFactory::GetVariable(unit_key, "Num_Chunks", 0);
}

CSVRow GetUnitRow(string filename, bool subu, int faction, bool readlast, bool &rread) {
std::string hashname = filename + "__" + FactionUtil::GetFactionName(faction);
for (int i = ((int) unitTables.size()) - (readlast ? 1 : 2); i >= 0; --i) {
unsigned int where;
if (unitTables[i]->RowExists(hashname, where)) {
rread = true;
return CSVRow(unitTables[i], where);
} else if (unitTables[i]->RowExists(filename, where)) {
rread = true;
return CSVRow(unitTables[i], where);
}
}
rread = false;
return CSVRow();
}

void Unit::WriteUnit(const char *modifications) {
bool bad = false;
if (!modifications) {
Expand All @@ -1167,7 +1127,9 @@ void Unit::WriteUnit(const char *modifications) {
}

std::map<std::string, std::string> map = UnitToMap();
std::string towrite = MapToJson(map);
boost::json::array json_root_array;
json_root_array.emplace_back(boost::json::value_from(map));
std::string towrite = boost::json::serialize(json_root_array);
f.Write(towrite.c_str(), towrite.length());
f.Close();
}
Expand Down
41 changes: 29 additions & 12 deletions engine/src/cmd/unit_json_factory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,12 @@
#include "unit_json_factory.h"
#include "unit_csv_factory.h"

#include <iostream>
#include <fstream>
#include <sstream>
#include <vector>
#include <string>
#include <map>
#include <boost/json.hpp>

#include "resource/json_utils.h"


void UnitJSONFactory::ParseJSON(VSFileSystem::VSFile &file, bool player_ship) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My code (deficiencies and all) assumed the following:

[
   {"a": "1", "b": 2},
   {"a": "1", "b": 2},
   {"a": "1", "b": 2},
   ...
]

However, this looks like it supports other file formats such as {"a": "1", "b": 2} without an array. Why would we do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that's how the Boost JSON serialization function constructs it. So when we go to load the file, we need to support either format -- with or without an array at the top level.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify further: Roy, your code saved an array with a single object element inside of it:

[
    {
        ...
    }
]

My code simply removes the root array:

{
    ...
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, we actually want the array. Right now the game saves each ship in its own file. There's no reason to do this. We just need to figure the whole player_ship issue. Either, replace it with player_ship_1...2...3 or player_ship_<random 6 alphanumeric> or something like that and check for prefix.

This would also solve the issue of not supporting a fleet containing two ships of the same type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.

const std::string json_text = file.ReadFull();
Expand All @@ -44,24 +40,45 @@ void UnitJSONFactory::ParseJSON(VSFileSystem::VSFile &file, bool player_ship) {
} catch (std::exception const& e) {
VS_LOG_FLUSH_EXIT(fatal, (boost::format("Error parsing JSON in UnitJSONFactory::ParseJSON(): %1%") % e.what()), 42);
}
boost::json::array root_array = json_value.get_array();

for(boost::json::value& unit_value : root_array) {
boost::json::object unit_object = unit_value.get_object();
std::map<std::string, std::string> unit_attributes;
if (json_value.is_object()) {
boost::json::object obj = json_value.as_object();
std::map<std::string, std::string> unit_attributes{};

for(boost::json::key_value_pair& pair : unit_object) {
for (boost::json::key_value_pair& pair : obj) {
const std::string value = boost::json::value_to<std::string>(pair.value());
unit_attributes[pair.key()] = value;
}

// Add root
unit_attributes["root"] = file.GetRoot();
unit_attributes["root"] = file.GetRoot();

if(player_ship) {
if (player_ship) {
UnitCSVFactory::units["player_ship"] = unit_attributes;
} else {
UnitCSVFactory::units[unit_attributes["Key"]] = unit_attributes;
}
} else if (json_value.is_array()) {
boost::json::array json_root = json_value.as_array();
for (boost::json::value & unit_value : json_root) {
boost::json::object unit_object = unit_value.get_object();
std::map<std::string, std::string> unit_attributes{};

for (boost::json::key_value_pair& pair : unit_object) {
const std::string value = boost::json::value_to<std::string>(pair.value());
unit_attributes[pair.key()] = value;
}

// Add root
unit_attributes["root"] = file.GetRoot();

if (player_ship) {
UnitCSVFactory::units["player_ship"] = unit_attributes;
} else {
UnitCSVFactory::units[unit_attributes["Key"]] = unit_attributes;
}
}
} else {
VS_LOG_FLUSH_EXIT(fatal, (boost::format("File '%1%' had an unexpected JSON structure. We don't know how to process it.") % file.GetFilename()), 42);
}

}
Loading