SoD files mod like

Seems that adding graphics to spells is too much for me… I got a few questions:

  1. Where in code are spell .def paired to indexes?
    1.1) How can I tell which def goes to which spell?

  2. Some spells have more than 1 def. How to handle those?

Bonus) Why are spell sounds added to spells in Cmusichandler? Must they stay there?

Bonus 2) I need a sort of game setting telling that it is the first time the game was run ever. So that the script will extract/move the necessary files to their corresponding places. Any ideas where I can save that?

  1. config/battles_graphics.json?

  2. Don’t know. Right now this is hardcoded.

Bonus) This is old behavior. Previously most of data was stored in appropriate handlers. To implement modding system this was changed to what we have now.
Feel free to move them into CSpell structure. And if you’ll send me patch with this change I’ll commit it to svn.

Bonus 2) Check if any of your files are present at their destination? At the very start of loading this can be detected by lack of config/settings.json file.

  1. ok now its obvious. Before I stopped reading at:
// WoG_Ac_format_to_def_names_mapping
"ac_mapping": ....
  • What does AC stands for?
  • I could not figure it out how/where to translate from AC ID to Spell ID?
  1. Spells with more than 1 def I will keep the existing vector storing, something like:
"graphics" : {"animation" :  "C15SPE0.DEF", "C15SPE1.DEF", "C15SPE2.DEF" ]}

Bonus 2) I was expecting to put this as a game_option somewhere in a “ini” file and at first run my scrip would change its value to false. This way If something goes wrong it could be easily/manually set to true.

Something from WoG/ERM. Not sure. May also represent H3 implementation.

Check spell_info.json. It has “graphics” field or something like that.

Bonus 2) You can use settings.json for this. All options along with default values described in schemas/settings.json.
Add your option in this file and you’ll be able to access it just like any other game options.
But player can always “reset” options by deleting this file…

These are the battl_graphics.json entries for Fire shield.

{ "id": 43, "defnames":  "C07SPF0.DEF", "C07SPF1.DEF", "C07SPF2.DEF", "C07SPF6.DEF", "C07SPF7.DEF", "C07SPF8.DEF" ] },
		{ "id": 44, "defnames":  "C07SPF0.DEF", "C07SPF4.DEF", "C07SPF5.DEF", "C07SPF9.DEF", "C07SPF10.DEF", "C07SPF11.DEF" ] },

How to put this into spell_info as “graphics” : “animation” entries?
there are 5 fireshield cases: *> < o / * aka(3 square convex/concave, 1 square, 2 squares left/right)

I want to add spell animations to CSpellHandler::CSpellHandler().

		//initialization of AC->def name mapping
		int ACid = spell.second"anim"].Float();
		std::vector< std::string > toAdd;

		for(const JsonNode &animation : spell.second"graphics"]"animation"].Vector()) 
		{
			toAdd.push_back(animation.String());
		}
		graphics->battleACToDef[ACid] = toAdd;
		
		graphics->spellEffectsPics = CDefHandler::giveDefEss("SpellInt.def");

The problem is that this is located in /lib and needs access to /client files. I cannot simply move it to /client since there are other /lib files that depend on this.

How to handle this particular case?
I remember something about “planned” future work in redistributing files from lib and client somewhat. Can anyone detail on that?

Why you need access to client files here?

What you need to do is to add several fields to CSpell structure (list of animation filenames, path to icon, etc.) and proper initialization of these fields from .json. This does not need access to Graphics.

Code that does actual load of data should be left at Graphics but it should use filenames stored in CSpell structure instead of reading configs directly.

Another noobish question (when you had it with me just let me know, it is OK, I am aware I am taking up your time).

I want to specify defs in spell_info.json. I did it the way you suggested above: read from .json in lib and use that info in client. Sadly I have some corrupted memory issues. Can anyone “logic proof” my intended changes? Included also the combined .json file for reference.
spell_graphics_from_json.zip (5.05 KB)

For me it works. Except for these error messages in console:
Spell << 81 already has a sound
Error: invalid sound for id 81
Error: invalid sound for id 11

BTW: I think you should remove spellSounds and initSpellsSounds() method from CSpellHandler entirely and move spell sound filenames into CSpell structure as well. You’re changing that code anyway.

First ty for trying it out. Strange that it crashes for me, when I want to cast, but I really do not know where else to look :(. I will make the sound change too and submit the new patch to you.

Spell 81 is the damage part of acid breath and since it is pointing to the same thing, it is correct that it has a sound already. I would say that we live it like this until someone fixes that.

  • Invalid sound for… is because LandMines and Damage Part of acid breath do not have sounds attached to them. In the sp_sounds.json they were just missing from the .json. Below the relevant code that expected that every spell has a sound. Do I leave it like it is or get rid of the check?
std::string sound = node.second"soundfile"].String();
			if (sound.empty())
                logGlobal->errorStream() << "Error: invalid sound for id "<< spellid;

I am thinking of using something like below for Force Field/FireWall. With hardcodded handling of array order. (Very much like in current code).

Any other suggestions?

		"forceField"     :
		{
			"id": 12,
			"name" : "Force field",
			"soundfile" : "FORCEFLD.wav",
			"effect": 0,
			"graphics" : {
				"animation" : 
					"C15SPE0.DEF", "C15SPE1.DEF", "C15SPE2.DEF",	// attacker basic; appearing / present / dissapearing
					"C15SPE5.DEF", "C15SPE4.DEF", "C15SPE5.DEF",	// defender basic; appearing / present / dissapearing ( missing appearing anim)
					"C15SPE6.DEF", "C15SPE7.DEF", "C15SPE8.DEF", 	// attacker expert; appearing / present / dissapearing
					"C15SPE9.DEF", "C15SPE10.DEF", "C15SPE11.DEF", 	// defender expert; appearing / present / dissapearing
				]
			},					
			"ranges":  "0", "0", "0", "0" ]
		},

I am trying to get rid of battleACToDef]. All that weird mapping makes no sense now that we can use “animation” : vector.

Edit: Ask me if this had unexpected consequences in other parts of the code (not spell related) :slight_smile:

Quick question: Why can’t I overload these? They have different number of parameters (not taking default values into account)

	void displayEffect(ui32 effect, int destTile, bool areaEffect = true); //displays default effect of a spell on the battlefield; affected: true - attacker. false - defender
	void displayEffect(ui32 spellID, ui32 animationIndex, int destTile, bool areaEffect = true); //displays specified effect of a spell on the battlefield; affected: true - attacker. false - defender

Int is convertible to bool and vice versa. Need to use strongly typed enums or pointers to do that, otheriwse it’s ambiguous.

Did not think about that :(. But TY for your quick answer. Luckily I can “force” destTile to be BattleHex instead of int. Problem solved.

Would it be possibble/ Fammiliar’s Mana Channel into 2 spells? Mana drain from opponent with graphics and sound followed by mana fill for your hero? Something like below? What I am thinking is for Familliars to “cast” these 2 spells one after the other.

(I could not find relevant Familliars call for CBattleInterface::spellCast())

		"magicChannelDrain" :
		{
			"id" : 85,
			"name" : "Magic Channel Drain",
			"soundfile" : "MAGCHDRN.wav", 
			"effect" : 0,
			"anim" : 76,
			"graphics" : {
				"animation" : "Sp07_B.DEF"]
			},
			"ranges" :  "0", "0", "0", "0" ],
		},
		"magicChannelFill" :
		{
			"id" : 86,
			"name" : "Magic Channel Fill",
			"soundfile" : "MAGCHFIL.wav", 
			"effect" : 0,
			"anim" : 75,
			"graphics" : {
				"animation" : "Sp07_A.DEF"]
			},
			"ranges" :  "0", "0", "0", "0" ],
		},

I would also like to add good/bad Morale and good/bad Luck as “spells”. In code to display their animations code to display custom Spells is used anyway. Any issues with that?

Mana channeling is NOT a spell and never was. The same is true for morale, luck, regeneration and other effects. The mere fact they have animation doesn’t mean they should be handled as spells, that code is already overcomplicated.

I just fixed all the spell issues after AVS rewrite, please don’t break anything again.

My 2 cents: Parts of that code NEED to go to mods. You have NO WAY around that. the longer you keep things hardcoded the more bugs and more work in the long run. I (or others) cannot do that without changes to the original sometimes awkward, incomplete or plain wrong code. (I commend the amount of effort everyone put in, but parts of code are far from perfect, for whatever legacy reasons)

If you do not trust my capabilities (probably because of the noobish questions I posted here), why not wait until I release the patch and veto it then. I have so-wie-so no access writes to SVN.

Now back to my “project”. Why again would Fammiliar ability not be a spell/creature ability like all others? You (and by you I mean VCMI team and possibly NWC) sure do treat it like a spell. Code-wise and resource wise.

Somewhat more questionable, but same goes for luck/morale. If you want to use methods from CSpellEffectAnimation and handle its animation toghether with the other spells or abilities, then why not make a dummy spell entry or maybe some similar minimalistic structure but in another file. (with only graphics and sound)?

I don’t. I have no idea how animations work, I only talk about mechanics.

Because it is NOT like others. Spells can be cast. Familiar ability can NOT be cast, it doesn’t have spell school, mana cost or any effect on its own. It doesn’t make sense at all, not sure where you are going here. Please explain what you are going to do and why?

Making thing not-hardcoded makes sense only if they are configurable. Again, what do you want to configure or modify? These effects are simple, one shot and have no parameters. But each of them is very specific and must be handled separately.