[Solved] A programming problem (handling configuration variables)

Firstly, your function doesn't "return" anything - but it probably should. Your interface here is trying to accomplish too much - it's simply called initVars, but it accepts any number of names, and tries to write out a reference to some changed vars (thus not performing any initialisation, so its name is a lie).

Suggested less obtuse interface

void setDefaultVars(CBlob@ this); //always called, so put this script first
void setVars(CBlob@ this, Variables@ vars); //overwrite the vars in some override script if wanted
Variables@ getVars(CBlob@ this);

Note that there's no "name" - I'd suggest you decide what name to use for this interface internally.

I'd also suggest finding a better name than "Vars", though in TR we just put this stuff inside a namespace ("Soldier") and called the structure "Data", so in external code it looks like Soldier::Data, and internally just looks like Data.

As for why the "pointer" doesn't get rewritten - you're not passing a handle to be rewritten.
"removing @ causes crash" is your hint here - you were trying to pass a reference to a function that expected a handle to modify. Your temporary handle (created by the @ operator) is being rewritten, then discarded.
It "might" work if you actually kept that handle around, and then used it to assign to your reference obj, but I'd change your interface rather than fix this confusing and badly named one.

 

I agree about the "quasi-c++" aspect, but remember that there's massive massive sandboxing issues with basically distributing a C or C++ compiler and C/++ source code as mods. It's totally non-viable at least in our case, and angelscript was the simplest option in terms of binding and porting code.

As for your approach - There's no reason to be using inheritance here! This may just be you coming from a very-OOP background or something but you dont need to create a new type in order to change some instance variables. Just create an actual descriptive constructor for your ZombieVariables type or modify a default-constructed instance.

Simply modifying a shared instance of a single type is even how the runner movement configuration works in vanilla. This gives tweakable variables between the migrants and the players (and could be used to create class movement differences if you wanted). It gives a config-file style init function which is trivial to edit.

Code:

void onInit(CMovement@ this) //NOTE: these days i'd just do this in the blob onInit tbh, CMovement is an artifact of years gone.
{
   RunnerMoveVars moveVars;
   //walking vars
   moveVars.walkSpeed = 2.6f;
   moveVars.walkSpeedInAir = 2.5f;
   moveVars.walkFactor = 1.0f;
   moveVars.walkLadderSpeed.Set(0.15f, 0.6f);
   //jumping vars
   moveVars.jumpMaxVel = 2.9f;
   moveVars.jumpStart = 1.0f;
   moveVars.jumpMid = 0.55f;
   moveVars.jumpEnd = 0.4f;
   moveVars.jumpFactor = 1.0f;
   moveVars.jumpCount = 0;
   moveVars.canVault = true;
   //(there are more, but you get the idea)
   this.getBlob().set("moveVars", moveVars);
}

Adding more variables in subsequent types like you have with your examples there (adding smell range, and then vision range) is not good practice for this kind of "configuration"/"shared store" - you'll have to constantly be casting between types and checking for nulls as a result of AS's dynamic casting, you pay a perf cost every time you do that, and composition (simply having separate types when you need more variables) will avoid all of those issues. You'll also run into headaches with shared types being necessary and believe me, they are bad headaches.

Please also note that there's no reason to wrap the set() call for what you want, and no reason to return something. The interface i provided above will give you simpler, easier to debug behaviour - and lets you change the dictionary name of your variables WITHOUT going and modifying every script that uses them, which is important for something "configurable".

That's why I would suggest not having them provide their own name to the set() call, and instead use a namespace to differentiate the SetVars calls - Creature::SetVars(someCustomCreatureVars) is plenty descriptive enough. As a bonus, the AS compiler can catch if you make a typo in the namespace - it can't check that your strings match.

 

The expense of casting comes from the additional null check and from the inconvenience to the programmer, its not terrible but it adds up and it makes your code less and less clear as time goes on.

Here's an actual concrete 3-file example showing defaulting and overriding something.

Code:

//UndeadCommon.as
namespace Undead
{
    //our shared data for all undead
    shared class Data
    {
        //dont be too shy to add more fields even if they're not used that much (or class specific)
        //for reference, we have over 100 members in the Soldier::Data structure in TR

        //the main advantages of this is speed and simplicity - you get the data once
        //and then do whatever with it, and you dont have to cast, or check for null as often

        //the main disadvantage is worse cache utilisation, which is average at best with AS anyway.
        // HOWEVER we're still only talking the order of 8 cache lines or so, and the data will stay
        // the same for 1 blob anyway.

        //consider - it'll matter if you have over a few hundred of them...
        //  but crimson wont handle that well anyway.

        //cached stuff
        //having this here saves getting it in every function
        //(also avoids call to engine as you're going to eat the get() call anyway)
        //if you REALLY want to have a bunch of separate structures, move this stuff into
        //some non-undead namespace (FrameCache would be an ok name for it)
        //but be careful that you actually need that flexibility
        int gametime = 0;

        //same goes for input
        bool left, right, up, down, action1, action2;
        bool just_left, just_right, just_up, just_down, just_action1, just_action2;

        //same goes for onground etc
        bool on_ground, on_wall, on_map, in_water;

        //timer stuff
        int attack_time = 30;
        int attack_timer = 0;
        int revive_time = 150;
        int revive_timer = 0;

        //ai stuff
        Vec2f last_target_pos = Vec2f_zero;

        //etc
    };

    //single point to change this if you ever need to
    const string _prop = "UNDEAD_DATA";

    //set the data - use this either just once (and then modify default)
    //          OR do a wholesale override in "modified" objects
    //           (imo the modify approach would be more flexible)

    //         (this takes by value to simplify passing default data
    //          as well as simplify using a generator function)
    void SetData(CBlob@ this, Data data)
    {
        this.set(_prop, @data);
    }

    //get the data - use this in every other script that needs
    //access to read or modify the shared data
    Data@ getData(CBlob@ this)
    {
        Data@ data = null;
        this.get(_prop, @data);
        return data;
    }

    void TickData(CBlob@ this, Data@ data)
    {
        //any default tick stuff
        //   updating any cached-for-this-frame vars (gametime, input vars...)
        //   resetting/updating timers + firing default cmds
    }

}

//UndeadBase.as
#include "UndeadCommon.as"
void onInit(CBlob@ this)
{
    Undead::SetData(this, Undead::Data());
}

void onTick(CBlob@ this)
{
    Undead::TickData(this, Undead::getData());
}

//Zombie.as
#include "UndeadCommon.as"
void onInit(CBlob@ this)
{
    Undead::Data@ data = Undead::getData(this);
    //zombies have a really slow attack
    data.attack_time = 60;
}

void onTick(CBlob@ this)
{
    Undead::Data@ data = Undead::getData(this);
    //do anything zombie-specific here
    //we dont need to TickData - its already been done
}

Mixins etc are nice if you want to use them, but we've had success with this approach in TR and its pretty simple.

 

Also Re: having it in a separate file - use the config files then? that's what they're for? Some sort of LoadUndeadData("path/to/configfilename.cfg") wouldn't be hard to cook up.

 

If you're asking the speed difference between this method, and storing each variable individually using the set/get properties methods. This method is much faster. If you are asking the speed difference between this and using inheritance. This is still faster because you drop the number of casts/null checks.

With regards to opening a config file, if your concern is you don't want people to have to edit script files to change values then a config file is your best (only?) option. The obvious problem you brought up is that the hdd read required by each onInit function is a bottleneck. However you can easily make some optimizations by simply caching values from a config in the rules when it's first read. This way if you create many instances of the same object only the first one will have a small penalty, which it does already if you are loading a new sprite.

 

No worries. I'm vaguely hoping it leads to more mods, particularly mods that the creators dont give up on because of development frustrations ;)

Rather than considering this as a generalisation of types, I'm honestly just "not thinking in terms of types" - I'm just storing all the data I need to do what I need to do. This is what I'm talking about with you thinking "too OOP" - the only point for this class is to store data. There's no reason that multiple types is somehow cleaner than one big type if they're all used for the same thing.
As I mentioned in the comments - if you start using this all over the place, then MAYBE pull the per-frame cached data out somewhere else.

In terms of properties we use a few (mostly just tags) for things that need to be accessed easily in other scripts (so they dont have to include SoldierCommon.as), or need to be synced more often (to avoid having a huge "data serialisation" packet that's filled out with a bunch of infrequently changing data) - but that's correct, we mostly avoid properties because they do an over-complicated memory access and string hashing and all sorts of performance-adverse stuff.

This is one of the main "death by a thousand cuts" pessimisations in regular KAG code, and something that we've learned from and worked around in TR.

No, the shared keyword means that the type definition is the same across multiple modules (scripts) - this is very important for any type you want to access in more than one script, or you'll potentially get nulls out the other side when the types dont match. This is somewhere C structures would be more useful, but we'll live.

Using shared DOES mean that you cant change the module without restarting the script engine though (which involves like quitting out to the menu and restarting sandbox or whatever, or restarting the game wholesale), so keep that in mind if you're using rebuild a lot.

You do have to name what variables to read, but that's the same as MyFancyVars.varname = whatever; - you just get a more idiot-friendly syntax for it all. As verra says you can cache the config variables in script-local variables with an init flag.

Code:

//flag that we have or haven't read from config
bool have_read_config = false;
string config_path = "example_vars.cfg"
//stuff to read from config
float examplevar;

void onInit(CBlob@ this)
{
	if(!have_read_config)
	{
		have_read_config = true;

		ConfigFile cfg = ConfigFile();
		cfg.loadFile(config_path);

		examplevar = cfg.read_f32("examplevar", 1.0f);
	}

	//set up the blob-local data here
}

 

I'm not sure why you think caching them in the script means you cant store them in some data structure inside the blob (or properties for that matter) (that's what the //set up the blob-local data here was about) - it just avoids hitting the disk every time you initialise a blob, which is what you expressed concern about.

I'd also as a rule try to avoid being too clever about configurability. If you want to later, you can always modify the code - solve the problems of today, not the problems that might or might not come around in 6 months time.

This isn't an excuse to blindly write spaghetti, but its also not a reason to blindly write spaghetti ;)

The blob design as it stands is rather awful :)
Yes, I'd mostly rather just use those hooks - you're indeed right that some hooks are easier to exclude on the server or when offscreen and so on (i think some stuff like this may happen for sprites, actually) - but when it comes to programmer convenience it's simpler to just think in terms of the actual object and what you want to do with it. And as you say, getBlob just expands the boilerplate section before you actually get to write any interesting ode.

The components should all be doing their own thing externally anyway and just require "direction" (animation changes, velocity changes, whatever) based on some object-specific behaviours, and those behaviours generally talk about the object at large anyway, so imo there's no reason to have component-specific hooks for everything. If they have some sort of events that they can produce and consume, these days i'd rather have an event management component that you can poke around in if you need to, rather than adding script invocations to the components and thrashing the cache.

We've still got sprite scripts in TR, and we've still got brain scripts (mostly out of habit i think), but nothing else. Sprite scripts at the very least are "semi-mandatory" at the moment if you want to render special stuff, though it could be handled after-the-fact in a rules script to cut down on script invocations...

Feel free to poke around the configs and source, most of it is a little cleaner than the KAG source. Exceptions are the AI stuff, menu code, and a few of the standalone items. The crosshair code isn't great either.

 

Re: best practices - it's something I've been thinking about writing for crimson, I think it'd be more valuable than the type interface docs in terms of value per hour, but I'm not sure how much we want to continue using this engine. It'd definitely help modders avoid making the same mistakes though.

MM may happily continue with it, but I'm done - I'd rather write things from scratch or use something off-the-shelf than continue trying to shoe-horn more games into its 8 (? 10?) year old frame.

 

I've gone through similar phases in my programming career. I've put together several game engines that have a very similar architecture to crimson, all though there was a whole lot less spaghetti. Even after making several "engines" and having had used them in their initial test projects, I've not bothered to use them for larger ones. There is a annoying quirk that comes with making new games with your engine if it isn't perfectly generic, that is that you have to go back and fix your engine every week to allow for xyz feature, slowly spaghettifying it. There's also some overhead with just trying to handle things in a generic manner (for scripting, or just architecture purposes). The biggest one that becomes a problem is networking. Networking works best, when you know exactly what the objects you are trying to send data about are. I wouldn't discourage you from trying to make your own engine, it's a good learning experience. However be careful when deciding how to architect your engine. Trying to do something "fancy" will more likely than not just hurt you in the long run. Crimson is a perfect example of this, my favorite is when I was tracking down a memory allocation error to do with the blob constructor. Which went through about 12 different files and ended in some deep dark dungeon (nested folder) that I had never seen before and used some "nice" stringifying lookuptable macro factory initialization mumbo jumbo that could have been simplified to new CBlob. Instead of poking it with a stick I left the beast to slumber. The KISS principle truly holds in game programming. It is often better and faster to just start from scratch and do things specific rather than generic.

A note on network programming. I used to think network programming was really scary. It's not easy that's for sure you have to do a lot of interesting things to keep everything straight and a lot more to make it smooth. However it definitely wasn't as bad as I thought. As long as you sit down and read a lot of things you can create something good, however it's not a task that works optimally with a generic setup.

/GAME ENGINE RANT/

 

Lets just say you don't know it like I do :) I'll don't plan on using it again for any project.

Re: "it works best if you know exactly what things you're talking about" - I'd actually argue that this is true for things other than networked programming too. You can always optimise best for the specific case. Generic solutions lead to generic improvements, and having to cater for the average case.

In the future, if I work with self-built tech, I'll be working towards more of a framework approach - have some helper code for things like rendering sprites, colliding shapes together and getting a resolution, tilemap stuff, polygon stuff, sounds, screen handling, and some generic network connection basics, and then build the systems actually needed for the project at hand.