Config reworked, is this better or worse?

[L3tum]

Reworked the config. Now mostly everything is done in ConfigManager and Config.cs is mostly only for serialization. Hopefully, this doesn't get commited to spaar/besiege-modloader...it shouldn't...

[spaar]

Two questions: 1) What exactly are the benefits of your reworked config over the current one? As far as I can see, the main differences are that one accesses a configuration value via ConfigurationManager instead of Configuration and that you need to add a new value in two places instead of one. 2) What does 22a5be1c1d6132bacdcb69edda21b6829099e9b6 have to do with the config?

[L3tum]

lol, for No.2, I don't know. I added it to my repository so I have a backup... I'll revert it. For No.1: I don't know why you should only need to add a new value in one place, since you still have to load it. I've found something I didn't look for, so I'll close this pr and rework it. As I said, I like prs because you can get feedback.

[spaar]

You're right, it's two places right now as well. But at least they are in the same (small) class.

[L3tum]

I'll try to rework it... I coded a mod for Minecraft like...3 years ago or so, and as I remember it's pretty easy to make a config, so I'll look into the code...Maybe I can implement it.

[spaar]

I'm not sure what you mean. I assume that creating a mod for Minecraft, there is some sort of API by the (quite advanced) mod loader (FML most likely)... We're currently just creating a configuration for the mod loader itself so I'm not sure if that will be particularly useful.

[L3tum]

I know, but the API is doing all the stuff and you just need to add the values. That's, what I think, we want to have.

[spaar]

Depends. An interface like that would be nice to have both for when the mod loader grows larger and for other mods to use a standard way of loading. Currently though, the loader itself is so small that it's not necessary and an API for mods can come in a future update. Or is that what you are trying to do right now?

[L3tum]

I don't know if I manage to do an API...but I don't like to....like adding something and not "finishing" it. It would be nice to have the Config finished and implemented, and then moving on to a new issue/feature.

[spaar]

I think a simple config for the mod loader is a very different feature to a very general config API for both the loader and mods. And the simple config is done right now (well, mostly, see #20) and works fine.

[L3tum]

Honestly, I don't like the current config. I don't know why, but I'll just try to work on it..

[ZiMMy]

How I imagine we may implement this: Firstly, a configuration class with fields only. Secondly, a static config manager class with the same fields as the configuration class and with all methods.

This way configmanager class can be accessed from any place of the code and do the job, while the configuration class is used for serialization/deserialization only.

[spaar]

@L3tum Well, you can do that, but I don't really want to change the config class without some tangible benefit.

@ZiMMy I'm curious, what's the benefit of doing that versus just having a couple of static methods inside the configuration class for (de-)serialization? Or non-static methods for that matter.

[ZiMMy]

We don't only need static methods, but static fields as well. The benefit is accessing the same static config manager class without creating an object to change values as it is done now.

[spaar]

Ah, I see. That makes sense.

[ZiMMy]

What do you think, spaar? I can try to remake configuration, but is it better to do this now, or postpone until the next release?

[spaar]

I think we should get the update out sooner rather than later. So it's probably fine to postpone it, as the current system should work for now.

[ZiMMy]

Okay. I'm also thinking about abandoning the console clear button idea, as it will look out of place and it is better to implement it as a console command in the future.

[spaar]

Implementing as a command is a good idea, yeah.

[L3tum]

@ZiMMy as I'm writing the command attribute atm, I'm planning on implementing some basic commands, like setting something in the mod loader or in Besiege....if you have some ideas tell me^^ Also, what I tried to do with this config was basically what you said with static things and stuff^^ You can still look at the changes, so it will probably a base for what you want to do.

[ZiMMy]

@L3tum I'll do. Well, nevertheless I think I can make it in a simplier way.

[L3tum]

@ZiMMy Thanks :) Hopefully^^ @spaar Do you want the commands in the current update? And what about the injector? I mean, which one should I work more on?^^

[spaar]

I think that it's quite possible that neither one ends up in this update. And that's fine. So, work on whichever one you prefer :)

[ZiMMy]

So, https://github.com/spaar/besiege-modloader/issues/23 and the release will be almost ready, I suppose. Oh, and someone with better knowledge of English please write the changelog )

[spaar]

Pretty much yeah. Don't worry, I'll write the changelog and package up the release once it's ready :) Btw: Do you think we should keep the practice that I currently have with the Releases folder? Or don't put those zip files etc. into git and use GitHub's releases feature? Or something else?

[ZiMMy]

IDK actually. Let's use releases feature maybe.

[spaar]

Yeah, I was thinking that too. Git isn't really made to handle binary files like that anyway, so it wasn't an ideal system, more like something I quickly did because I wanted to keep old versions around ^^