[READY] Effects Update 1

[i2amroy]

And it's ready for testing! Basically a huge background expansion to JSONize the effects system, as well as a near total migration from the old diseases system over to the new effects system. Don't be fooled by the line counts though, the majority of changes were simply replacing "X_disease(" with "X_effect(" on a huge number of lines. The important things to be looking at are the changes in effect.cpp, effect.h, and the process_effects(), has_effect(), add_effects(), and move_effects() functions. As such it should merge relatively painlessly.

The several effects that I wasn't able to convert over are now placed in "hardcoded_effects()", where they should function identically to how they did before the move. Overall there were a few small changes to comply with some of the limitations of the json fields, but the majority of effects remained identical in function to how they were before.

This is also a huge step towards having effects and diseases work on everything, players, monsters, and npc's included.

I've also drawn up some documentation for the way the new effects json file is formatted and included it in the PR. It can be found here.

There will probably be at least one bug that I missed somewhere, but I tried to squash as many as I could find, so hopefully it's not as bad as some other PR's have been.

Fixes #9101. Fixes #7449. Closes #7532.

[BevapDin]

That writes an integer as key of the json object, but keys should be strings, can the parse read this back in?

Edit: I see you have overladed the read function, so it works. But the key of json objects should be string according to some json format definition.

[i2amroy]

Ah, that makes sense. I'll take a look at fixing it.

[BevapDin]

The function modifies the effects map (if bp == num_bp), thereby invalidating the maps iterator, that you use here. You could change effects.erase( eff_id ); in remove_effects to only clear the map effects[eff_id].clear().

That would also be consistent with the other case (bp != num_bp) where it might leave an empty entry in the effects maps (effects[eff_id] still exists, but is an empty map, the iterator maps points to this empty map and is still valid).

Edit : this causes a crash on my build.

[BevapDin]

Out of curiosity: why are there two body parts used here? One is used for the environmental protection check and the other is the actual effected body part. Why should it be possible for example to attack the left leg and (if it succeeds) infect the right arm?

[i2amroy]

The idea was so that, for example, you could create a poison that could get into your body through any given attack location, but then target a specific body part (or be generalized). This is already used for tons of things like smoke, where the protecting body part is the mouth but the effect itself actually targets num_bp.

[kevingranade]

This is begging to be pulled out into a helper:

void extract_effect( JsonObject &j, std::string mod_type,  std::pair<int, int> &mod_value)
{
    if(j.has_member(mod_type)) {
        JsonArray jsarr = j.get_array(mod_type);
        mod_value.first = jsarr.get_int(0);
        if (jsarr.size() >= 2) {
            mod_value.second = jsarr.get_int(1);
        } else {
            mod_value.second = mod_value.first;
        }
    }
}

[i2amroy]

Updated the "effects" field in the item use JSON field and added a similar type of field for successful monster attacks (i.e., damaged the player successfully). I've updated one monster (the fungal fighter) as a proof of concept but don't plan to update any others in this PR (since it would just make testing/reviewing more difficult and merging compatibility harder to maintain for little gain). I've also updated the documentation to reflect this fact.

That was pretty much the last feature I wanted to get in in this PR (since part of having a modding system for effects is the ability to apply them), so unless someone sees a problem this will probably be identical to the final merged version (excepting occasional merges to the keep it up to date of course).

[Lain-]

Pardon me pestering: New stable is out, let's get this in soon?

[KA101]

Sure, just needs updated and tested. Not sure where i2amroy is.

[i2amroy]

I've been taking a bit of a break until the new stable hit. I'll look into updating this tonight.

[i2amroy]

And updated, just need someone to triple-check to make sure it works and merge it. :)

[KA101]

Saw your name being pulled off the core-dev credit list and went D8 .

[i2amroy]

To be fair I haven't exactly been exercising my privileges as a github manager recently. :P Don't worry though, I'm definitely still here and still planning on contributing now that the feature freeze is off. :D

[i2amroy]

I'll just keep fixing merge conflicts every day or so on this till it get's merged. :P