Proposal for creature config

HellKiller · August 19, 2012, 3:01pm

I think that is stupid idea. Why?
Because modding and scripting base on shortcuts. What if i want to add 30 new creature? I cant copy/past and modify some things. I think creatures and all object still should base on id, because is easy solution, and it isnt annoying for modders. But version with id must be preper on edit, should be without any limits and stupid solutions.

I think, it should be something like is now but authors should add command, which can easily change statistic, without edit ZCRTRAIT. And command with sounds and will be good.

itoijala · August 19, 2012, 3:33pm

The format was based on a couple of requirements which I found important:

no numerical ids. They make combining mods impossible because they might want the same id. This is improbable with names. Names are also easier to read. You always know which creature is meant. With ids you have to look up the id.
support multiple files with creatures. This is important for mods, otherwise mods need to include all of the creatures from the original game if they want to add one creature. Some of the original formats do not include an id. Instead they use the order in the file. This is impossible if you want to have many files with creatures. The old formats make combining mods impossible.
flexible but backward-compatible. This means that VCMI can add new optional attributes for creatures but old files still work. This can be done with json. If the attribute is missing just use the default value. This is not possible using the original formats because they are based on a table and adding columns is not possible.
good support for default values. The above includes all the attributes to specify the format. No creature will need all of the attributes.
easy to mod existing creatures. This would be supported by only including the attributes you want to change. If you only want to change the Pikeman’s attack to 10, just make the file:

{
	"Pikeman" :
	{
		"attack" : 10
	}
}

Here is the same Pikeman without the comments and using a slightly modified format (a bit smaller). The cost is the only thing that lost flexibility. It is now not possible to define new resources (metal, …) because the number of resources is fixed to 7.

{
	"Pikeman" :
	{
		"names" : "Pikeman", "Pikemen"],
		"level" : 1,
		"faction" : "Castle",
		"upgrades" : "Halberdier",
		"cost" : [0, 0, 0, 0, 0, 0, 60],
		"fightValue" : 100,
		"aiValue" : 80,
		"growth" : 14,
		"hitPoints" : 10,
		"speed" : 4,
		"attack" : 4,
		"defence" : 5,
		"damage" : [1, 3],
		"amm" : [20, 50],
		"abilities" :
		{
			"CHARGE_IMMUNITY" : {}
		},
		"defFile" : "CPKMAN.DEF",
		"animationTimeWalk" : 1.15,
		"soundAttack": "PIKEATTK.wav",
		"soundDefend": "PIKEDFND.wav",
		"soundKilled": "PIKEKILL.wav",
		"soundMove": "PIKEMOVE.wav",
		"soundWince": "PIKEWNCE.wav"
	}
}

For reference, here is everything about the Pikeman in the old formats (I probably missed some things…):

{
	"name": "Pikeman",
	"attack": "PIKEATTK.wav",
	"defend": "PIKEDFND.wav",
	"killed": "PIKEKILL.wav",
	"move": "PIKEMOVE.wav",
	"wince": "PIKEWNCE.wav"
}

1.00	1.15	1.00	1.00	0	0	0	0	0	0	0.0	0.0	0.0	0.0	0.0	0.0	0.0	0.0	0.0	0.0	0.0	0.0	0	0	Pikeman

{
	"id": 0,
	"level": 1,
	"name":  "Pikeman" ],
	"faction": 0,
	"upgrades": [1],
	"ability_add":   "CHARGE_IMMUNITY", 0, 0, 0 ] ], 		//pikeman immunity to Champion charge bonus
	"defname": "CPKMAN.DEF"
}

0	S	+	0	0	0	0	1	1	1	1	1	1	1	Pikeman: Speed R4
0	f	B	2	2	2	2	2	1	1	1	1	1	1	Pikeman: 2-hex (Breath) Attack R5
0	h	#66	0	0	0	2	2	3	3	4	4	5	6	Pikeman: Hatred Dmg-Black Knight
0	h	#67	0	0	0	2	2	3	3	4	4	5	6	Pikeman: Hatred Dmg-Dread Knight
0	f	f	0	0	0	0	0	1	1	1	1	1	1	Pikeman: Fearless R5

Pikeman	Pikemen	0	0	0	0	0	0	60	100	80	14	0	10	4	4	5	1	3	0	0	20	50	"Immune to Champion charge bonus."	0

Warmonger · August 19, 2012, 3:44pm

Looks like “double-wide” property is missing. It defines whether creature occupies 1 or 2 hexes on battlefield.

I would also add optional vector of stack experience bonuses. Overall, this feature is already supported and new creatures should be allowed to use it easily

As to abilities, I’m going to create complete parser for Bonuses and all their properties. It will be necessary sooner or later.
Removing abilities is not really necessary here, I guess it’s just because of inconsistent original config files.

Also, HellKiller, I didn’t really understand your post.

itoijala · August 19, 2012, 4:03pm

oops…

I did not do the WoG properties because I do not really understand them. It should be easy to add them though…

I think it is necessary because a modder might want to remove an ability from an existing creature. I think what can be done without scripting should be done without scripting. It makes everything easier to understand and debug. Of course I am not against a scripting API to all of this :).

Do you prefer the original version from the beginning of the thread (ignoring the commented-out bits) or the smaller version? I think the original is nice and logical but the newer one is smaller and also not bad. Though the cost inflexibility is not nice…

It should be relatively easy (meaning: I could try this in the next couple of days. Perhaps making the game load the creatures and then output them) to create files in this format for SoD and a “mod” file for WoG (not a real mod, just put WoG stuff in separate files to make separating VCMI from WoG easier). If a script is made then support for the old formats could be removed someday because mods could be automatically ported.

EDIT:

I suggest a system where each ability has a handler that is called to handle the ability. It would also parse the arguments. This way the arguments can be anything needed for the ability and would not included not needed things. The creature parser could then call the correct handler based on the name of the ability. Later scripts could add new handlers…

HellKiller · August 19, 2012, 4:22pm

I want to say that this is good idea, but need some changes.
Adding and changing creatures should base on ID, no on names.

{ 
    "Pikeman" : 
    { 
        "names" : "Pikeman", "Pikemen"], 
        "level" : 1, 
        "faction" : "Castle", 
        "upgrades" : "Halberdier", 
        "cost" : [0, 0, 0, 0, 0, 0, 60], 
        "fightValue" : 100, 
        "aiValue" : 80, 
        "growth" : 14, 
        "hitPoints" : 10, 
        "speed" : 4, 
        "attack" : 4, 
        "defence" : 5, 
        "damage" : [1, 3], 
        "amm" : [20, 50], 
        "abilities" : 
        { 
            "CHARGE_IMMUNITY" : {} 
        }, 
        "defFile" : "CPKMAN.DEF", 
        "animationTimeWalk" : 1.15, 
        "soundAttack": "PIKEATTK.wav", 
        "soundDefend": "PIKEDFND.wav", 
        "soundKilled": "PIKEKILL.wav", 
        "soundMove": "PIKEMOVE.wav", 
        "soundWince": "PIKEWNCE.wav" 
    } 
}

This version looks very good, but NAMES should be replace by ID.

Warmonger · August 19, 2012, 4:45pm

We already had a long discussion about that. Have a look here and here.

Not sure if you are familiar with Bonus System. Have a look here, here and just everywhere in the code.

itoijala · August 19, 2012, 5:35pm

Perhaps there should be an area in the wiki with formats like this one. One area for proposals and one for recommendations from the VCMI team. This means that a format would become a recommendation when it has been discussed, agreed upon by the team and implemented in the code.

Tow · August 19, 2012, 5:40pm

itoijala, thank you for sharing your input. It’s interesting and well-thought.

It’s a range used to pick quantity of monster on adventure map (if it wasn’t set by mapmaker).

Correct.

I agree, type of turret shooter should be property of town, not creature.

Numerical, sequential IDs shall be given to creatures by engine when the game starts and it should be treated as engine internal implementation detail. If modmaker wants to reference a particular creature, string identifier has to be used.
Otherwise… it would be a disaster when having multiple mods.

We could provide support for both formats, that is allow resources to be both vector of integers or struct {restypes -> resQuantities}. Or even accept a single integer and interpret is as a cost in gold. We can have both brevity and flexibility.

Good idea, I’d say we need it pretty much right now.
Artifact bonuses should be moved to configs. It’d best if you write C++ Bonus to JSON formatter and auto-generate that configs.

itoijala · August 19, 2012, 5:48pm

There are not many creatures (only level 7?) that cost other resources than gold. I think it is best if an integer means gold and otherwise you have to use the long format. That way most creatures are short but there is full flexibility to change the number and names of resources. This means that one resource (gold) must be hard-coded into the game. But its name in the interface and icons would be changeable and every total-conversion needs at least one resource, so that is not a problem. Then we have the best of both worlds.

itoijala · August 20, 2012, 4:14pm

I created a page in the wiki for the “current” version of the proposal. It should be updated when the proposed format changes. This way it is easy to see the current proposal. The page includes a short todo list and both long and short versions of the proposed format. The Pikeman is given as an example of both.

P.S. hope it was ok to create a wiki page.

P.P.S I realized that the original map editors will not be able to read the format. They should still work because the original data is also there but a project to create a new map editor should be started (probably better to discuss this in another thread).

EDIT: Link to wiki page wiki.vcmi.eu/index.php?title=Creature_Format

Ivan · August 21, 2012, 9:40am

I don’t think that we can start new editor now. We should finish main game first.

"names" : "", ""]

Why not split them into namePlural\nameSingular?

"upgrades" : "Halberdier",

What about multiple upgrades? Current code already have support for them.

//            "ext1" : "",              // default none, "" 
 //            "ext2" : "",              // default none, "" 
 //            "moveStart" : "",         // default none, "" 
 //            "moveEnd" : ""            // default none, ""

IIRC these two are identical - several creatures have EXT1-EXT2 files used on movement start\end.

Long version looks much more organized IMO.

Data present in original H3 txt’s should remain there. Reorganizing our json configuration system is OK. Idea is:

for H3 creatures: read H3 txt first, then fill missing pieces with data from .json
for creatures from mods: read everything from .json

itoijala · August 22, 2012, 5:49pm

I think it should not be an ability. The abilities are supposed to become hooks into the bonus system and doubleWide is not a bonus. That will just make the code more complicated. It should just be a normal attribute.

I had this in the version from the first post. I changed it because HellKiller said it was too long. Shall I put the first version back?

I don’t really know how the attribute is used. What does it mean to have multiple upgrades? We could just use a vector of strings.

Ivan:

//            "ext1" : "",              // default none, "" 
 //            "ext2" : "",              // default none, "" 
 //            "moveStart" : "",         // default none, "" 
 //            "moveEnd" : ""            // default none, ""

IIRC these two are identical - several creatures have EXT1-EXT2 files used on movement start\end.

I added both because both are in the code. If they are always the same then ext1/2 should be removed.

EDIT: I changed the wiki page to reflect your comments.

I think the code can become much cleaner if the converter approach is used. There is no problem distributing the config files (unlike the artwork). Reading the old files will also make making WoG optional much harder.

An optional mode that loads the old formats could be made (command-line switch?) to support other mods (is this even possible with the old formats?). But long-term it is a very good idea to move away from and deprecate the old formats, they are just too inflexible.

A bit off-topic: I think that the code should be refactored so that loading is separate from the model layer. Many of the files are too big for my taste. I think one class per file is a good idea so that the code remains navigable. For example CCreatureHandler.cpp/.h should be split into CCreature.cpp/.h and CCreatureLoader.cpp/.h (and other parts?). When the loading code is separate from the model it becomes easier to support multiple formats.

Warmonger · August 22, 2012, 6:17pm

I can see that doubleWide is not an ability in a code, it’s not handled by Bonus System at least. Could you be more specific about it?

Ivan · August 22, 2012, 8:45pm

There is at least one problem with redistribution of configs: localization. And it can’t be separated from configuration: some of files have both mechanics values as well as localized texts.

WoG optionality shouldn’t be a problem. Creature loading may work like this:

for each entry in json config
- load creature data with same ID (if present) from H3 txt’s
- load data from json, overwriting any H3 data

Modding shouldn’t be a problem either - if you want to change value from H3 txt’s you may as well add it to json config and it will override setting from txt’s.

Yes. Vector of identifiers. This is how it works right now (creatures.json).

And one more thing: Instead of

{
    "pikeman" : { ... }
}

I think we should use this:


    {
        "id" : "pikeman",
        ...
    }
]

Why? Our Json parser uses sorted storage for keys (std::map). As result creatures will be added in game in alphabetical order ignoring order in file. So it is better to store this as array to prevent confusion or some weird issues.

Tow_dragon · August 23, 2012, 6:54am

Oh, you are right. I was 100% sure there is DOUBLE_WIDE bonus. There isn’t, and has never been one. Strange. It looks more like oversight than design decision. Is anyone against fixing it?

Warmonger · August 23, 2012, 6:57am

I see no point in DOUBLE_WIDE being a Bonus. How can you imagine creature becoming normal or wide in a middle of battle? It’s just a fixed property related to size of graphics.

itoijala · August 23, 2012, 7:49am

Ivan:

And one more thing: Instead of
{
    "pikeman" : { ... }
}
I think we should use this:
    {
        "id" : "pikeman",
        ...
    }
]
Why? Our Json parser uses sorted storage for keys (std::map). As result creatures will be added in game in alphabetical order ignoring order in file. So it is better to store this as array to prevent confusion or some weird issues.

I would say that it is a good thing that the order from the file is not used. Modders should not depend on that implementation detail. And when mods are implemented and many creature config files are used then the order is not so well defined anyway. Loading them in a (for the modder) random order is the best way to avoid bugs caused by relying on the order in the file. The order has to be irrelevant. Otherwise the modder has to be very careful to not refer to ids that come later in the file.

TL;DR: Modders should not rely on the order anyway, only on the loading order of entire mods. Loading order inside a mod should be undefined and an implementation detail.

I changed the wiki to reflect the comments on doubleWide and upgrades.

Warmonger · August 23, 2012, 8:22am

Ok, I guess you can create a parser now and see how it works

Tow_dragon · August 23, 2012, 8:40am

There are already many static bonuses, i.e. KING1 or CATAPULT. I see no point in making an exception for DOUBLE_WIDE. Not to mention that some mod maker would want to make this property non-static for some reason.

It’s better to relay on order consciously than being surprised that it’s the cause of a bug. I think order should be specified and modders should be advised not to use this specification without really good reasons. The C and C++'s history has shown that undefined behavior can lead to nasty bugs.

Ivan · August 23, 2012, 8:42am

itoijala,
There will be issues with existing content - how VCMI can load maps from WoG if load order (and as result - numeric ID’s) is undefined? Same goes to original game as well as for such mods like HoTA.
And there are may be some more issues of which I am not aware right now.