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

add extension to write to cba_settings.sqf file #1333

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Binary file added cba_settings.dll
Binary file not shown.
Binary file added cba_settings_x64.dll
Binary file not shown.
335 changes: 335 additions & 0 deletions extensions/cba_settings/cba_settings.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,335 @@
#include <iostream>
#include <fstream>
#include <string>
#include <algorithm>
#include "trim.h"

#define VERSION "1.0.0"

#define RET \
strncpy_s(output, outputSize, result.c_str(), _TRUNCATE);\
return

#define RETURN(val)\
strncpy_s(output, outputSize, result.c_str(), _TRUNCATE);\
return (val)

#define SYNTAX_ERROR_WRONG_PARAMS_SIZE 101
Copy link

@TheMagnetar TheMagnetar May 10, 2020

Choose a reason for hiding this comment

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

since dedmen is suggesting c++17 features (string_view, filesystem) it is also advisable to use for numerical data the following:

Suggested change
#define SYNTAX_ERROR_WRONG_PARAMS_SIZE 101
static constexpr int32_t SYNTAX_ERROR_WRONG_PARAMS_SIZE = 101;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's the benefit over

const int SYNTAX_ERROR_WRONG_PARAMS_SIZE = 101;

?

Choose a reason for hiding this comment

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

constexpr is evaluated at compile time

Copy link
Contributor

Choose a reason for hiding this comment

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

which won't make any difference here as the end result.

#define SYNTAX_ERROR_WRONG_PARAMS_TYPE 102
#define PARAMS_ERROR_TOO_MANY_ARGS 201
#define EXECUTION_WARNING_TAKES_TOO_LONG 301

#define TEMP_FILE "userconfig/temp"
#define SETTINGS_FILE "userconfig/cba_settings.sqf"
#define DELIM ":"
#define MAX_CALLBACK_LENGTH 10000

using namespace std;

Choose a reason for hiding this comment

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

This is not recommended in general and it is rather dangerous.


string version() {
return VERSION;
}

string clear() {
ofstream file;
file.open(SETTINGS_FILE, ios::out);
file.close();
return "true";
}

string load(int strlen) {
Copy link
Contributor

Choose a reason for hiding this comment

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

strlen is the name of a function, please don't use that as variable name.

Copy link
Contributor Author

@commy2 commy2 May 8, 2020

Choose a reason for hiding this comment

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

Haha. Oops.

string result = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

reserve.

result.reserve(std::filesystem::size(SETTINGS_FILE));

Otherwise you resize/reallocate/copy almost every time you append a line.

Copy link
Contributor Author

@commy2 commy2 May 8, 2020

Choose a reason for hiding this comment

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

Cool, didn't know filestream::size is a thing. Though, is it reasonable to allocate ~10k characters (bytes?) at once?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well more reasonable than first allocating 16, then deleting and reallocating 32, then deleting and reallocating 64, then deleting and reallocating 128, then deleting and reallocating 512, then deleting and reallocating 2048, then deleting and reallocating 8192, then...


ifstream file;
file.open(SETTINGS_FILE, ios::in);

string line;
while (file.good()) {
getline(file, line);
if (file.good() || line.length() != 0) // Don't explode file by appending duplicate additional newlines.
result = result + trim(line) + "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
result = result + trim(line) + "\n";
result.append(trim(line) + "\n");

}

file.close();

if (strlen * MAX_CALLBACK_LENGTH > result.length())
Copy link

@TheMagnetar TheMagnetar May 10, 2020

Choose a reason for hiding this comment

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

Not writing { } after if/while etc is not recommended at all. Consider the following

if (strlen * MAX_CALLBACK_LENGTH > result.length())
    do_stuf()
do_other_stuff

and then one decides to comment do_stuff

if (strlen * MAX_CALLBACK_LENGTH > result.length())
    // do_stuf()
do_other_stuff

resulting in do_other_stuff being inside the if context breaking a lot of stuff

return "";

result = result.substr(strlen * MAX_CALLBACK_LENGTH, MAX_CALLBACK_LENGTH);
return result;
}

string parse(int strlen) {
string result = "[";

ifstream file;
file.open(SETTINGS_FILE, ios::in);

string line;
while (file.good()) {
getline(file, line);
line = trim_left(line);

// Ignore commented out lines.
if (line.substr(0, 2) == "//")
Copy link
Contributor

Choose a reason for hiding this comment

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

This copies the string every time. Would be WAY more efficient to use a string_view here, as that is all that you need. For trim_left too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also why trim left and right seperately? why not immediately trim both sides?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why not immediately trim both sides?

    force force   setting    =    123;

is something one could write. If I process this, I need multiple trims, but not on both sides all the time.

continue;

// Count force tags.
int force = 0;
while (line.substr(0, 6) == "force ") {
force++;
line = line.substr(6);
line = trim_left(line);
}

// Get setting.
size_t i = line.find("=");
if (i == string::npos) // Ignore ill-formed line.
continue;

string item = line.substr(0, i);
item = trim_right(item);

// Get value.
string value = line.substr(i + 1);
value = trim(value);

while (value.back() == ';')
value.pop_back();

value = trim_right(value);

result = result + "[\"" + item + "\"," + value + "," + to_string(force) + "],";
}

file.close();

result.pop_back();
result.push_back(']');

if (strlen * MAX_CALLBACK_LENGTH > result.length())
return "";

result = result.substr(strlen * MAX_CALLBACK_LENGTH, MAX_CALLBACK_LENGTH);
return result;
}

string read(string setting) {
string result = "[\"\",false,-1]";

ifstream file;
file.open(SETTINGS_FILE, ios::in);

string line;
while (file.good()) {
getline(file, line);
line = trim_left(line);

// Ignore commented out lines.
if (line.substr(0, 2) == "//")
continue;

// Count force tags.
int force = 0;
while (line.substr(0, 6) == "force ") {
Copy link
Contributor

Choose a reason for hiding this comment

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

you are copying the substring on every iteration here. If you use the string_view trim, it will only move a pointer around

force++;
line = line.substr(6);
line = trim_left(line);
}

// Get setting.
size_t i = line.find("=");
if (i == string::npos) // Ignore ill-formed line.
continue;

string item = line.substr(0, i);
item = trim_right(item);

if (_stricmp(item.c_str(), setting.c_str()) == 0) {
// Get value.
string value = line.substr(i + 1);
value = trim(value);

while (value.back() == ';')
value.pop_back();

value = trim_right(value);

result = "[\"" + item + "\"," + value + "," + to_string(force) + "]";
break;
}
}

file.close();
return result;
}

string append(string setting, string value, int force) {

Choose a reason for hiding this comment

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

Passing by reference is advisable with classes. It avoids copying the whole class. The same can be applied to the other functions.

Suggested change
string append(string setting, string value, int force) {
string append(const string &setting, const string &value, int force) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool. Didn't know it copies, and that this is how one prevents it.

ofstream file;
file.open(SETTINGS_FILE, ios::app);

string line = "";
while (force-- > 0)
line = line + "force ";

Choose a reason for hiding this comment

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

Suggested change
line = line + "force ";
line.append("force ");


line = line + setting + " = " + value + ";";

file << line << endl;
file.close();
return "true";
}

string write(string setting, string value, int force) {
string result = "false";

ofstream temp;
temp.open(string(TEMP_FILE).c_str(), ios::out);
Copy link
Contributor

Choose a reason for hiding this comment

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

you are reloading, reparsing, rewriting the whole file for every setting property write?
You could just parse it once, and store it in memory. Your extension won't get unloaded

Copy link
Contributor Author

@commy2 commy2 May 8, 2020

Choose a reason for hiding this comment

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

store it in memory

How?

Copy link
Contributor

Choose a reason for hiding this comment

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

Comment below, just a global variable. Array or map or some container


ifstream file;
file.open(SETTINGS_FILE, ios::in);

string line;
while (file.good()) {
getline(file, line);
line = trim_left(line);
string orig = trim_right(line);

if (line.length() == 0 || result == "true") {
if (file.good()) // Don't explode file by appending duplicate additional newlines.
temp << orig << endl;

continue;
}

// Keep commented out lines.
if (line.substr(0, 2) == "//") {
temp << orig << endl;
continue;
}

// Strip force tags.
while (line.substr(0, 6) == "force ") {
line = line.substr(6);
line = trim_left(line);
}

// Get setting.
size_t i = line.find("=");
if (i == string::npos) // Remove ill-formed line.
continue;

string item = line.substr(0, i);
item = trim_right(item);

if (_stricmp(item.c_str(), setting.c_str()) == 0) {
// Updade matching lines.
line = "";
while (force-- > 0)
line = line + "force ";

line = line + setting + " = " + value + ";";

temp << line << endl;
result = "true";
} else {
// Keep non-matching lines.
temp << orig << endl;
}
}

temp.close();
file.close();

if (result == "true") {
remove(SETTINGS_FILE);
rename(TEMP_FILE, SETTINGS_FILE);
} else {
remove(TEMP_FILE);
result = append(setting, value, force);
};

return result;
}

extern "C" {
__declspec(dllexport) void __stdcall RVExtensionVersion(char* output, int outputSize);
__declspec(dllexport) void __stdcall RVExtension(char* output, int outputSize, const char* function);
__declspec(dllexport) int __stdcall RVExtensionArgs(char* output, int outputSize, const char* function, const char** args, int argsCnt);
}

void __stdcall RVExtensionVersion(char* output, int outputSize) {
strncpy_s(output, outputSize, VERSION, _TRUNCATE);
}

void __stdcall RVExtension(char* output, int outputSize, const char* function) {
string result = "false";

if (_stricmp(function, "version") == 0) {
result = version();
RET;
}

RET;
}

int __stdcall RVExtensionArgs(char* output, int outputSize, const char* function, const char** args, int argsCnt) {
string result = "false";

if (argsCnt > 3) {
RETURN(PARAMS_ERROR_TOO_MANY_ARGS);
}

string setting = "";
if (argsCnt >= 1) {
setting = args[0];
if (setting.substr(0, 1) == "\"") // Strip extra quote marks.
setting = setting.substr(1, setting.length() - 2);
}

string value = "";
if (argsCnt >= 2)
value = args[1];

int force = 0;
if (argsCnt >= 3)
force = min(stoi(args[2]), 2);

if (_stricmp(function, "version") == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why even check the whole string?
c
l
p
r
a

the first character would be sufficient for what you need

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True I guess, but what does it matter? This is not something that needs to be particularly fast.

Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't really matter, just my "every nanosecond counts" bullshit.

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 understand.

result = version();
RETURN(0);
}

if (_stricmp(function, "clear") == 0) {
result = clear();
RETURN(0);
}

if (_stricmp(function, "load") == 0) {
int strlen = stoi(setting);
result = load(strlen);
RETURN(0);
}

if (_stricmp(function, "parse") == 0) {
int strlen = stoi(setting);
result = parse(strlen);
RETURN(0);
}

if (_stricmp(function, "read") == 0) {
result = read(setting);
RETURN(0);
}

if (_stricmp(function, "append") == 0) {
result = append(setting, value, force);
RETURN(0);
}

if (_stricmp(function, "write") == 0) {
result = write(setting, value, force);
RETURN(0);
}

RETURN(1);
}
33 changes: 33 additions & 0 deletions extensions/cba_settings/trim.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#include <string>

using namespace std;

const string WHITESPACE = " \n\r\f\t\v";

string trim_left(string str) {
size_t first = str.find_first_not_of(WHITESPACE);

if (first == string::npos)
return "";
else
return str.substr(first);
}

string trim_right(string str) {
size_t last = str.find_last_not_of(WHITESPACE);

if (last == string::npos)
return "";
else
return str.substr(0, last + 1);
}

string trim(string str) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Example for a more efficient trim that doesn't copy the string
https://github.com/dedmen/armake/blob/cpp/src/utils.cpp#L240

size_t first = str.find_first_not_of(WHITESPACE);
size_t last = str.find_last_not_of(WHITESPACE);

if (first == last)
return "";
else
return str.substr(first, last - first + 1);
}