MegaGlest Forum

MegaGlest => Feature requests => Topic started by: tomreyn on 2 October 2013, 17:27:08

Title: Replace attack-strenght by attack-strength over time
Post by: tomreyn on 2 October 2013, 17:27:08

As any developer and modder is aware of, there is this misspelled XML tag attack-strenght in the code, which apparently has been around since the old Glest days. Sadly, until today we're making all modders conform with this misspelled tag because just replacing it in the code would break backward compatibility.

So what I'm suggesting is to do the following instead:

• Replace all occurrences of attack-strenght by attack-strength in the engine source code
• Add attack-strenght as an alias for attack-strength for backward compatibility
• Replace all occurrences of attack-strenght by attack-strength in all techtrees released with the engine (i.e. Megapack)
• Optionally (might again break backward compatibility so think twice, but then mod availability can be limited to certain engine versions) replace all occurrences of attack-strenght by attack-strength in all mods available on the game mod database (mod menu)
• Report a deprecation warning if attack-strenght is found in a techtree during validation and loading
• Replace all occurrences of attack-strenght by attack-strength in the documentation

Sadly I can't tell much work "add an alias" really involves, it may sound easier than it's done. GAE seems to have done it (https://forum.megaglest.org/index.php?topic=7056.msg72199#msg72199), so maybe this approach can be reused.

Title: Re: Replace attack-strenght by attack-strength over time
Post by: MoLAoS on 2 October 2013, 18:09:31

Starting at line 765 of skill_type.ccp take out this:

765    attackStrength= sn->getChild("attack-strenght")->getAttribute("value")->getIntValue();

And add this:

const XmlNode *attackStrengthNode = sn->getChild("attack-strength", 0, false);
if (attackStrengthNode) {
    attackStrength = attackStrengthNode->getAttribute("value")->getIntValue();
} else {
    attackStrength = sn->getChild("attack-strenght")->getAttribute("value")->getIntValue();
}

Problem solved.

Title: Re: Replace attack-strenght by attack-strength over time
Post by: softcoder on 3 October 2013, 14:52:14

Fixed in svn rev #: 4595

Title: Re: Replace attack-strenght by attack-strength over time
Post by: tomreyn on 4 October 2013, 01:50:07

Thanks to both of you!

While this is the most important change, this is only the second step of this long list I had in mind, though. What do you think about the other suggestions?

Title: Re: Replace attack-strenght by attack-strength over time
Post by: MoLAoS on 4 October 2013, 12:58:28

AFAIK, only the one XML defining thing has the misspelling. The others were probably fixed already because only that single instance actually affected modders.

As far as changing mods, that's a simple search-replace run on each thing, so one for source, assuming you actually still need to change something there, one for megapack, and then one for each other mod you are fixing. Shouldn't take more than 20 minutes.

Title: Re: Replace attack-strenght by attack-strength over time
Post by: tomreyn on 4 October 2013, 13:31:13

r4596 has a few more occurences. I am happy to fix the .pl code but would prefer not to touch the .cpp.

Code: [Select]

~/SCM/megaglest-trunk$ grep -rIi attack-strenght source/
source/tools/convert_faction_xml2html/convert_faction_xml2html.pl:        $attack_strenght{$u}  = get_value($xpath, "/upgrade/attack-strenght" );
source/tools/convert_faction_xml2html/convert_faction_xml2html.pl:                    $attack_strenght{ $s }   = get_value($xpath, "/unit/skills/skill/name[\@value='$skill']/parent::*/attack-strenght/\@value");
source/glest_game/types/skill_type.cpp:    attackStrengthXMLTags.push_back("attack-strenght");
source/glest_game/types/upgrade_type.cpp:    attackStrengthXMLTags.push_back("attack-strenght");
~/SCM/megaglest-trunk$

I can do the Megapack if this seems like a good idea? There should be 153 changes there unless awk failed me:

Code: [Select]

~/SCM/megaglest-trunk$ grep -rcIi attack-strenght data/glest_game/ | awk -F: 'BEGIN {COUNTER=0}; {if ($NF != 0) {COUNTER = COUNTER + $NF}}; END {print COUNTER}'
153
~/SCM/megaglest-trunk$

I'm also happy to update the wiki once all code supports the new variant. We won't be able to fix any mods the mod DB refers to unless we are happy to store two variants of all techtrees for backwards compatibility, though. But I guess we can live with this (a deprecation warning would help mod developers prevent to reuse the misspelled tag in future mods).

Title: Re: Replace attack-strenght by attack-strength over time
Post by: MoLAoS on 4 October 2013, 15:12:15

Oh, I forgot that MegaGlest didn't abstract all that stuff. In GAE the enhancement class is the only one that says attack-strength/attack-strenght and upgrades and skills and units have an enhancement member inside them. So you only need the one change.

What are the .pl files for? I don't recognize those from GAE.