Difference between revisions of "Talk:Object instance format"

From VCMI Project Wiki
Jump to: navigation, search
(Notes)
(Notes)
 
(10 intermediate revisions by 3 users not shown)
Line 2: Line 2:
 
* Mine : "army" - already supported by engine, but can't be configured in map
 
* Mine : "army" - already supported by engine, but can't be configured in map
 
*: Should we prepare army/guards option for _all_ objects?
 
*: Should we prepare army/guards option for _all_ objects?
 +
*:: Only those that can have army in H3. Mines can have guards set by player.
 +
 
* Random dwelling : instead of using "linked" field just don't save unused field. So if dwelling is linked "sameAsTown" is present, while "allowedFactions" is not and vice versa.
 
* Random dwelling : instead of using "linked" field just don't save unused field. So if dwelling is linked "sameAsTown" is present, while "allowedFactions" is not and vice versa.
 +
 
* Random dwelling : Perhaps we can find better way to store references to other object without specifying coordinates? By some unique ID for example? (same applies to quests and what else uses such scheme)
 
* Random dwelling : Perhaps we can find better way to store references to other object without specifying coordinates? By some unique ID for example? (same applies to quests and what else uses such scheme)
 
*: We can introduce instance ID for all objects as incremented counter (vector position will not work for map editor) (and may be reuse it in engine too as separate std::map<si32, CGObjectInstance>).
 
*: We can introduce instance ID for all objects as incremented counter (vector position will not work for map editor) (and may be reuse it in engine too as separate std::map<si32, CGObjectInstance>).
Line 8: Line 11:
 
* Scholar : save only active reward? Scholar with skill will have "skill" : "wisdom", scholar with spell will have "spell" : "magicArrow" field
 
* Scholar : save only active reward? Scholar with skill will have "skill" : "wisdom", scholar with spell will have "spell" : "magicArrow" field
 
*: Ok {{done}}, may we need to unify rewards with other objects?
 
*: Ok {{done}}, may we need to unify rewards with other objects?
 +
*:: For example? If you mean using same code for giving rewards - then I suggest to wait.
 +
 
* Monster : remove "randomCount" - if "count" is not defined, assume that count is random
 
* Monster : remove "randomCount" - if "count" is not defined, assume that count is random
 
*: {{done}}
 
*: {{done}}
Line 18: Line 23:
 
* Stack instance: "creID" -> "type" - readability
 
* Stack instance: "creID" -> "type" - readability
 
*:{{done}} Hate this one: "type" is a keyword in pascal (but can be used as identifier in form &type).
 
*:{{done}} Hate this one: "type" is a keyword in pascal (but can be used as identifier in form &type).
 +
*:: In cases like this I'm fine with any other word that makes sense :)
  
 
* Garrison: "garrison" -> "army" - for consistency with hero, town, etc.
 
* Garrison: "garrison" -> "army" - for consistency with hero, town, etc.
Line 24: Line 30:
 
* Hero: "sex" (string) -> "female" (bool) - to match engine & modding format
 
* Hero: "sex" (string) -> "female" (bool) - to match engine & modding format
 
*: This need 3 values: female, male, and "defined by config"
 
*: This need 3 values: female, male, and "defined by config"
 +
*:: true, false and null? Null is valid value for json and so is lack of entry.
 +
*::: The question is how to define this in code cleanly? Simple types are not nullable. (I can use Variant, and recheck how is it serialized).
 +
 
* Hero: "id" - I don't like this word. Type? Base Hero?  
 
* Hero: "id" - I don't like this word. Type? Base Hero?  
 
*:{{done}} set to type, see above :)
 
*:{{done}} set to type, see above :)
 +
 
* Check for case inconsistencies - I see "spellID" and "spellId" in some fields. Same applies to other types of id's
 
* Check for case inconsistencies - I see "spellID" and "spellId" in some fields. Same applies to other types of id's
 
* Or just remove "id" part from all fields. "spell" : "slow". Looks fine for me.
 
* Or just remove "id" part from all fields. "spell" : "slow". Looks fine for me.
*:{{wip}}
+
*:{{done}}
  
 
== Notes ==
 
== Notes ==
  
 
* IIRC there is difference between "empty" garrison in town and "undefined" garrison. Game spawns 1-2 stack in town for undefined garrisons. For format that means that entry with 0 items is not identical to lack of entry.
 
* IIRC there is difference between "empty" garrison in town and "undefined" garrison. Game spawns 1-2 stack in town for undefined garrisons. For format that means that entry with 0 items is not identical to lack of entry.
:* {{todo}}
+
:* Let array of empty objects be empty garrison, array of zero elements be undefined garrison.
  
 
* Any reason to have any actual default values? What "mana" : -1 means for hero? If default value should be used then just don't write this entry in map. (or write null entry: "mana" : null).
 
* Any reason to have any actual default values? What "mana" : -1 means for hero? If default value should be used then just don't write this entry in map. (or write null entry: "mana" : null).
:* where did you fine that? In pandora or local event? There it means take 1 mana. Default there in 0 and not saved (examples was made with explicit serialization of default values)
+
:* where did you fined that? In pandora or local event? There it means take 1 mana. Default there is 0 and not saved (examples was made with explicit serialization of default values). Also serialize simple types as null is not trivial.
 +
::* Check hero format - it has several "-1". Not mana but patrol radius & primary skills. As for serialization - prehaps you use something similar to c++ boost::optional?
 +
:::* In real save they will be null, all default values are omitted by serializer and default equals null on de-serialisation. I`ll fix examples.
 +
 
 +
* I don't see a reason to have this field:
 +
<syntaxhighlight lang="javascript">
 +
"upgraded": true  //optional, for random objects
 +
</syntaxhighlight>
 +
Upgrades of stacks are fixed depend on object coordinates. Check CGCreature::containsUpgradedStack() --[[User:Warmonger|Warmonger]] ([[User talk:Warmonger|talk]]) 12:03, 5 February 2016 (CET)
 +
: This is not for CGCreature but for CArmedInstance. See CArmedInstance::randomizeArmy(). --[[User:AVS|AVS]] ([[User talk:AVS|talk]]) 11:08, 7 February 2016 (CET)

Latest revision as of 10:08, 7 February 2016

Possible enchancements

  • Mine : "army" - already supported by engine, but can't be configured in map
    Should we prepare army/guards option for _all_ objects?
    Only those that can have army in H3. Mines can have guards set by player.
  • Random dwelling : instead of using "linked" field just don't save unused field. So if dwelling is linked "sameAsTown" is present, while "allowedFactions" is not and vice versa.
  • Random dwelling : Perhaps we can find better way to store references to other object without specifying coordinates? By some unique ID for example? (same applies to quests and what else uses such scheme)
    We can introduce instance ID for all objects as incremented counter (vector position will not work for map editor) (and may be reuse it in engine too as separate std::map<si32, CGObjectInstance>).
  • Scholar : save only active reward? Scholar with skill will have "skill" : "wisdom", scholar with spell will have "spell" : "magicArrow" field
    Ok [Done], may we need to unify rewards with other objects?
    For example? If you mean using same code for giving rewards - then I suggest to wait.
  • Monster : remove "randomCount" - if "count" is not defined, assume that count is random
    [Done]

Fields renames/tweaks

  • Stack instance: "creCount" -> "amount" - readability
    [Done]
  • Stack instance: "creID" -> "type" - readability
    [Done] Hate this one: "type" is a keyword in pascal (but can be used as identifier in form &type).
    In cases like this I'm fine with any other word that makes sense :)
  • Garrison: "garrison" -> "army" - for consistency with hero, town, etc.
    [Done]
  • Hero: "sex" (string) -> "female" (bool) - to match engine & modding format
    This need 3 values: female, male, and "defined by config"
    true, false and null? Null is valid value for json and so is lack of entry.
    The question is how to define this in code cleanly? Simple types are not nullable. (I can use Variant, and recheck how is it serialized).
  • Hero: "id" - I don't like this word. Type? Base Hero?
    [Done] set to type, see above :)
  • Check for case inconsistencies - I see "spellID" and "spellId" in some fields. Same applies to other types of id's
  • Or just remove "id" part from all fields. "spell" : "slow". Looks fine for me.
    [Done]

Notes

  • IIRC there is difference between "empty" garrison in town and "undefined" garrison. Game spawns 1-2 stack in town for undefined garrisons. For format that means that entry with 0 items is not identical to lack of entry.
  • Let array of empty objects be empty garrison, array of zero elements be undefined garrison.
  • Any reason to have any actual default values? What "mana" : -1 means for hero? If default value should be used then just don't write this entry in map. (or write null entry: "mana" : null).
  • where did you fined that? In pandora or local event? There it means take 1 mana. Default there is 0 and not saved (examples was made with explicit serialization of default values). Also serialize simple types as null is not trivial.
  • Check hero format - it has several "-1". Not mana but patrol radius & primary skills. As for serialization - prehaps you use something similar to c++ boost::optional?
  • In real save they will be null, all default values are omitted by serializer and default equals null on de-serialisation. I`ll fix examples.
  • I don't see a reason to have this field:
"upgraded": true  //optional, for random objects

Upgrades of stacks are fixed depend on object coordinates. Check CGCreature::containsUpgradedStack() --Warmonger (talk) 12:03, 5 February 2016 (CET)

This is not for CGCreature but for CArmedInstance. See CArmedInstance::randomizeArmy(). --AVS (talk) 11:08, 7 February 2016 (CET)