Remove dependency of OSYS on ConfigAdv

[lennyhans]

The file ConfigAdv.cpp has a reference to OSYS.h and OSYS.cpp that could be removed, because the only use is on the line 161

int ConfigAdv::load(char *filename)
{	
FilePath full_path(sys.dir_config);
(...)

So in the constructor that path can be "injected" as value, so there's no need to reference the sys instance

The benefit?, prevent recompile on changes of osys ( if was make as a lib, for example) and can help to decouple a little bit (it's something)

[J-Moravec]

this is not necessarily true. When you are hunting for these things, you need to check what functions are using ConfigAdv::load. For example, there is another function in the OSYS.cpp that uses it: ConfigAdv::init(), so you wouldn't really solve anything with the proposed change, only introduced additional dependencies elsewhere (such as into OTUTOR.cpp).

ConfigAv::load path handling is the same as other load functions. There is a lot of repeated code, so maybe there is a way to abstract that.

You can quickly check these things by typing:
grep -n "load(" src/*

[lennyhans]

I got it, but I don't mean with the method, I mean the "constructor", but sure there is changes.

Anyways the change help to separate the file from the main one.

[J-Moravec]

So, correct me if I understand you correctly. You are proposing the following changes:

int ConfigAdv::load(char *filename) -> int ConfigAdv::load(FullPath *path)

int ConfigAdv::init() -> int ConfigAdv::init(FullPath *path)

?

Then there would be problem with:

sys.show_error_dialog(error_msg)

Personally, I find the way it is now easy to read although I applaud the attempt to remove dependencies. If anything, it feels to me that OSYS is bloated and could be split into some sort of settings class.

I am also not against abstracting some code in the various load functions.

[lennyhans]

Not really, but similar.

this could be
int ConfigAdv::load(char *filename) -> int ConfigAdv::load(const char* file_path, char *filename)

or this
int ConfigAdv::init() -> int ConfigAdv::init(const char* file_path)

But after reading you, can be a good idea to make a a specific struct to manage files

[the3dfxdude]

But after reading you, can be a good idea to make a a specific struct to manage files

If you are speaking of a struct that lists the locations of all the data file types, then that is what Sys is. The purpose of Sys is to bring up the core services of the game. One of which is to be able to find the files the game needs to run.

If you isolate ConfigAdv from Sys, then you have ConfigAdv, which conveys settings from the user to adjust deep parts of the game, not able to directly see the core aspects of the game, where pretty much everything game specific does already. You may have a point if File or FileTxt was accessing Sys to find the file it was going to read, but those classes are isolated already. I'd argue that ConfigAdv should be able to ask the game directly where its file resides, as every File/Res based game object does.

If anything, it feels to me that OSYS is bloated and could be split into some sort of settings class.

Maybe Sys is bloated, but on splitting this up here what settings are really in this class? The data file paths? This is set up runtime based on OS implementation and basically the first thing that needs to happen. I would consider this hard-coded on necessity and not really a setting intended for users, even though there is an override for testing purposes.

If anything, Sys is everything that needs to talk to the OS, so knowing where things are at, and that there is video, audio, error dialogs and the like does make sense. Yes Sys blurs the line between the Game classes, but I don't think any game outside of 7k really needs Sys, so splitting things here might be quite low in return.

[lennyhans]

No, I mean a struct with the generic data file management, like path, name, type.

But okay, I don't want to trim sys, just try to decouple it, so AM say to SYS (or the rest of the classes) how to do a particular way.

Now that I see better my suggestion I was thinking on the libwebservice, no advconfig, anyways the idea still apply.

[lennyhans]

I will close this, as there is no need to leave it open