MegaGlest Forum
MegaGlest => Feature requests => Topic started by: Omega on 22 July 2014, 01:22:13
-
I intend to re-implement a feature of GAE that I believe was useful: tags.
Tags are just a label (or multiple labels) that can be given to a unit type. Then we can make changes that only apply to units that have certain tags. For example, instead of listing all the units that an upgrade is applied to, we could instead use tags to assign some custom property (eg, "warrior" or "mage") and then apply the upgrade to all units with that property. These are more useful with attack boosts, since attack boosts can affect multiple factions.
Tags are most effective if they are used consistently throughout the entire techtree. They make it easy to apply attack boosts to a group of units based on some property (such as whether they're a building, a sword user, etc).
Tags also have future potential in being used with other features. For example, looting could be improved by assigning a list of tags for units that can loot (perhaps normal units can loot, but buildings (like the defense tower) cannot). In fact, I intend to extend looting in this way after tags are implemented.
I intend to use the same syntax as GAE did (http://docs.megaglest.org/XML/Unit). As an aside, the feature does seem redundant when first using it, but it's actually quite a please to use (speaking from experience) when used consistently throughout the techtree. It works best for mods that use attack-boosts (emanations in GAE) heavily. It's a pain in the ass to keep adding units to the various affects lists of attack-boosts every time you create a unit. Easier to add a tag in one spot.
For a real example of how a tag is useful, consider some zombie warrior that inflicts a disease (using an attack boost) on nearby units (in GAE, this would be better done with an effect (http://docs.megaglest.org/GAE/Effects), which is essentially an attack boost that is applied to the target of an attack). Anyway, it doesn't make sense to apply a disease to a building, so naturally we're going to limit this attack boost to only effect humanoid units. How do we do this? Do we keep an affects-list of every humanoid unit in this attack boost (and bear in mind we have to manage all these affects-lists)? Tags provide an alternative by simply saying that the attack boost affects "humanoids", and then each humanoid can be given a "humanoid" tag. Thus, we localize the unit properties (being humanoid) to the location that they are relevant.
It's going to be a few days (or perhaps until the weekend) until I can get started with this, so feel free to discuss this idea.
-
I *** LOVE IT!.
I miss this! This feature is so nessesary, and should work for targeting too.
i vote yes.
A way to show what tags a Building has would be nice too.(optionable on a per Building per tag basis). so you can chose only to show some tags. so you can create mechanics that rely on tags and use tags for show and hidden mechanics.
-
This feature is so nessesary, and should work for targeting too.
Could you elaborate on what you mean by "targeting"?
-
im not sure it's entirely the same, but basically, removing and adding targets to a skill, for example. a skill that does acid damage does not effect Buildings but does effect biological units, but not fx mechanical units. so 3 tags named "Building" "Mechanical" "biological"
a attack skill.
<valid-targets target"=biological">
<non-valid-targets target="building""mechanical">
Does this makes sense.
-
That's simple enough on its own, but because it affects the AI, it's a bit more complicated.
Might be something worth looking into in the future (although at the time, I have little experience with AI).
-
Awesome. :thumbup: you rock omega
-
im not sure it's entirely the same, but basically, removing and adding targets to a skill, for example. a skill that does acid damage does not effect Buildings but does effect biological units, but not fx mechanical units. so 3 tags named "Building" "Mechanical" "biological"
a attack skill.
<valid-targets target"=biological">
<non-valid-targets target="building""mechanical">
Does this makes sense.
I thought this would be for attack boosts or upgrades and not for attacks because the things your descripe there can be done already. Its called attack-multiplier and uses the armor of the units. You can also say that the unit will only take half the damage or something. Maybe we could add that you can upgrade all units with a specific armor-type. Or can tags and attack-multiplier exists at once? This is not that simple I think, because we need to plan exactly for which things we use those tags.
I think having both could be possible but would confuse players.
-
I thought this would be for attack boosts or upgrades and not for attacks because the things your descripe there can be done already. Its called attack-multiplier and uses the armor of the units. You can also say that the unit will only take half the damage or something. Maybe we could add that you can upgrade all units with a specific armor-type. Or can tags and attack-multiplier exists at once? This is not that simple I think, because we need to plan exactly for which things we use those tags.
I think having both could be possible but would confuse players.
This is NOT The same. This makes unit's untargetable by the unit and the damage is simply not applied to units with tags.
Armor is for migation of damage across different units and making some units stronger even tho they have different health portions.
Basically Armor makes a unit Tougher
What i suggest makes you unable to hit a unit.
Again it is NOT the same
-
At the moment I'm having build errors on the current develop branch (without any changes) and have not been able to get any assistance with these.
So I'm putting this on the backburner for now. It's no fun to develop for a program that I can't even reasonably get to compile. If I'm able to get the code to work with me in in the future instead of against me, I'll take another look at implementing this. But in the meantime, this is just a major pain in the ass. Thus, I will not be making any more changes to MegaGlest's game code.
EDIT: Got it working now. As per the advice of titi, the build folder (in mk/linux) has to be removed. It seems that the issue is related to CMake.
-
Are these Tags just some kind of shortcuts so you don't have to list all effected units and just write the tag?
If it is like this and it needs many changes in the code I don't think its needed too much. If its added it should be just made as a shortcut while loading the techtree. Inside the loaded types the unittypes should be explicitlly listed like now because it keeps things simpler( and I think faster ) in the game
-
No, They can work like that, but the purpose is classifying units.
-
yes, but thats only useful if you use the classification. And in moment you use it it's the same.
-
and not creating the function in the first place is suposed to help?. Classifying a unit based on unit-type is important.
I will bet you money that this function is going to be worth GOLD.
i know that as a creator it feels good to have control with what's going on but dont strangle this man, Just because you can't see the need or use for it, others can, and we can use it.
-
The one thing about tags is that you need to create the mechanic to utilize the tag. So tags may be of use to modders using scripting, but less valuable to people who can't/don't script.
Tags may be more useful if things are put directly into the engine, such as managing AI and other stuff. I have no idea how far Omega plans to take tags though.
-
Molas brings up some additional ideas on how tags could be useful. A Lua function could provide access to tags (so you could, eg, have an area that damages humanoid units).
With that said, I do think tags simplify the process of assigned affected units. Many types of upgrades affect categories of units. Used consistently, assigning units to a category ("tag") in one place (their unit XML) removes the worry of creating inconsistencies (by failing to change every other place that requires this change). It's fairly well established that the more places that have to be updated to fully implement a change, the more complicated it is to create the change (evolutionary coupling aka logical coupling).
-
With that said, I do think tags simplify the process of assigned affected units. Many types of upgrades affect categories of units. Used consistently, assigning units to a category ("tag") in one place (their unit XML) removes the worry of creating inconsistencies (by failing to change every other place that requires this change). It's fairly well established that the more places that have to be updated to fully implement a change, the more complicated it is to create the change (evolutionary coupling aka logical coupling).
Exactly. Also, if you want an aura that effects units from other factions (whether friend or foe), without tags you'd have to every possible unit in the tech tree that could be affected, whereas with tags it's much easier. If you want the aura to affect human units, you just specify that tag, and then it kinda takes care of itself.
-
What John.d.h says is the first thing that convinces me ;) . I am just not sure how many changes this will need.
-
What John.d.h says is the first thing that convinces me ;) . I am just not sure how many changes this will need.
The sad thing is that what i said was the exact same thing as Jhon.D, Just in another way...
Im happy Jhon. D got the point across but im a little sad it's so difficult to communicate in general.
-
What John.d.h says is the first thing that convinces me ;) . I am just not sure how many changes this will need.
The sad thing is that what i said was the exact same thing as Jhon.D, Just in another way...
Im happy Jhon. D got the point across but im a little sad it's so difficult to communicate in general.
Its probable that Titi is simply given John undue weight in discussions for reasons that have little to do with the validity of the specific points in question.
-
The sad thing is that what i said was the exact same thing as Jhon.D, Just in another way...
Im happy Jhon. D got the point across but im a little sad it's so difficult to communicate in general.
I think I emphasized more the cross-faction capabilities and how much work it would save in that regard, which isn't really a message that I gleaned from your posts. If it's there, I missed it. If I do better at communication, it's because I have a ton of practice at it, and we're dealing in my native language.
Its probable that Titi is simply given John undue weight in discussions for reasons that have little to do with the validity of the specific points in question.
If you're implying favoritism, my track record with feature suggestions in MegaGlest suggests otherwise. This might be the first one he's ever agreed with. ;D
-
The sad thing is that what i said was the exact same thing as Jhon.D, Just in another way...
Im happy Jhon. D got the point across but im a little sad it's so difficult to communicate in general.
I think I emphasized more the cross-faction capabilities and how much work it would save in that regard, which isn't really a message that I gleaned from your posts. If it's there, I missed it. If I do better at communication, it's because I have a ton of practice at it, and we're dealing in my native language.
Its probable that Titi is simply given John undue weight in discussions for reasons that have little to do with the validity of the specific points in question.
If you're implying favoritism, my track record with feature suggestions in MegaGlest suggests otherwise. This might be the first one he's ever agreed with. ;D
Its not favoritism exactly. Its more that he respects you even if he disagrees most of the time. Most people don't base their decisions in discussions on the points made but who makes them. Its not something specific to Titi or you.
-
John.d.h said that those tags would work across factions ( and maybe even techtrees so you can move factions form other techtrees in and out if they work with the same tags ) . This was a very important point.
-
and maybe even techtrees so you can move factions form other techtrees in and out if they work with the same tags
Since tags are merely strings, yes, they can be moved into other factions. Of course, this does have the downside of no "type checking" (if you typo a tag, you won't be warned), but this is a useful tradeoff (and not the only thing that isn't checked).
-
and maybe even techtrees so you can move factions form other techtrees in and out if they work with the same tags
Since tags are merely strings, yes, they can be moved into other factions. Of course, this does have the downside of no "type checking" (if you typo a tag, you won't be warned), but this is a useful tradeoff (and not the only thing that isn't checked).
You could make a tag list for the tech tree and check tags in the rest of the XML against that. That will prevent types pretty easily. Its barely any extra effort.
-
I support the idea of tags for attack boosts. Both Annex and Terra would benefit greatly from them.
-
So, tags have been implemented, but I'm trying to improve the change so that in the list of affected units, you'll just see the tag name instead of the numerous units that have that tag. Thus, we have a set of units affected by the upgrade and a set of tags that are affected by the upgrades. Unfortunately, I'm faced with a strange bug that I'm not able to work around.
It's summed up in this StackOverflow post (http://stackoverflow.com/questions/25101181/dereferencing-a-set-iterator-causes-a-seg-fault), but someone here may have a better idea of what could be wrong. For context, the changes up till this are on GitHub (https://github.com/MikeHoffert/megaglest-source/commits/feature-tags).
I don't see anything that should have gone out of scope and there's no undefined behavior that I'm aware of (unfortunately, the nature of undefined behavior in C++ makes it difficult to be sure). Compiler isn't warning me about anything. I haven't had any success with a debugger, mostly due to not being used to debugging C++ (the STL classes are almost unreadable).
Any help would be appreciated.
-
I don't think you provided enough examples to find an error, but why not just use vectors or for_each? Its not like you are calling the code enough to be a performance issue.
-
I used sets to simplify the handling of duplicates (no need to manually check for duplicates -- sets ignore those). The original implementation found all unit types with a tag and threw them in the affects vector. Now that tags are separate from the affects, I guess it's not so needed. Still a strange issue, though.
Regarding `for_each`, I've never really cared for it. I'd love to use the C++11's range based for loops, but they require MSVS 2012 or later and I'm not sure if that's supported, yet. Although I'd have to check with Softcoder regarding the use of newer language features, anyway. Wouldn't want to break the build on some platform.
At any rate, the solution I used was a conditional to check if the set of unit tags is empty. Although I still don't understand why the inner for loop was even entered. The issue boiled down to `tags.empty()` being true while `tags.begin() != tags.end()` is also true (but it shouldn't be!).
-
As someone more or less mentioned on stackoverflow: Change getTags() to return a reference, not a value/copy.
const set<string> &getTags() const {return tags;}
Be aware that the return type is const, so use a const iterator.
Not sure if that's all, but you don't want a (deep) copy there, for sure. The iterator gets out of bounds because you check against the end of a different set. Every call to getTags() gets its own copy.
-
Hey Yggdrasil, long time, no see. Anyway, thank you for your reply. That worked perfectly. While I knew about passing by reference and value, I did not know that return types had similar semantics. The code is now working as expected.
Anyway, now that that's working as expected, I just a have a few things related to tags that I need to finish up. I need to verify that saving and loading the game works as expected, I want to refactor AttackBoost::isAffected (it has a horrible amount of code clones), and I want to make tags translatable (in the same manner as the rest of the techtrees).
EDIT: This makes me realize that there's a number of other places where we return a copy to a vector instead of a reference. They haven't been caught because they didn't use iterators like I did, but pose a threat creating of future bugs. Might be worth taking the time to convert these.
-
Hey Yggdrasil, long time, no see. Anyway, thank you for your reply. That worked perfectly. While I knew about passing by reference and value, I did not know that return types had similar semantics. The code is now working as expected.
Anyway, now that that's working as expected, I just a have a few things related to tags that I need to finish up. I need to verify that saving and loading the game works as expected, I want to refactor AttackBoost::isAffected (it has a horrible amount of code clones), and I want to make tags translatable (in the same manner as the rest of the techtrees).
EDIT: This makes me realize that there's a number of other places where we return a copy to a vector instead of a reference. They haven't been caught because they didn't use iterators like I did, but pose a threat creating of future bugs. Might be worth taking the time to convert these.
How does saving work in MegaGlest? I spent a lot of time modifying GAE's save and I wonder if GAE and MG do it in the same way.
-
Done! Tags are working as expected. Rather than repeat myself, please see the pull request here (https://github.com/MegaGlest/megaglest-source/pull/25). It details everything you need to know about tags, including syntax and translating.
How does saving work in MegaGlest? I spent a lot of time modifying GAE's save and I wonder if GAE and MG do it in the same way.
It dumps pretty much everything you'd need to know about the game state (eg, list of all units, stats that they have at a given time, etc) into an XML. When loading the game, that XML is read to determine what to load (eg, what units to load, their stats, etc). It largely works in a hierarchy, having factions call load methods of units, units call load methods of attack boosts, etc. The code is very messy, though. There's a great deal of commented out code. I think that's mostly because who ever implemented the save/load feature realized they don't have to save everything. Upgrade types aren't saved or loaded, for example, since they can't be changed in game, so can be loaded from the faction (although the upgrade manager and its list of in-progress/completed upgrades is saved/loaded).
EDIT: For the convenience of who ever has to merge this change, I added additional comments to explain design choices and places that aren't very clear in the diff. These are (as far as I know) only visible on GitHub (https://github.com/MegaGlest/megaglest-source/pull/25/files).
EDIT2: For further convenience, I created a pull request (https://github.com/MegaGlest/megaglest-data/pull/6) for the megaglest-data change (just a single line in a language file).
-
This is merged into branch develop now. please test!
-
I assume this feature has not been tested for a long time. I want to try this, but where's the code?
I have read this entire thread already, but still no code. Also, watch the language.
-
Did you see this too? https://github.com/MegaGlest/megaglest-source/pull/25
btw @Ishmaru: die you use this yet ?
-
Sorry, I didn't click all the links in this thread. I had to skim through this thread looking for the code.
I'll place the code here so I don't need to click any links again.
"Tags are assigned with the <tags> element, which is a child of <parameters> in the unit XML. Units can have any number of tags. Tags are merely strings. Note that tags are case sensitive. The <tags> element is optional and may be empty."
"Tags should be restricted to letters, numbers, and underscores. As with units, underscores will be converted to spaces and the first letter of each word capitalized."
<parameters>
<!-- ... -->
<tags>
<tag value="warrior"/>
<tag value="burnable"/>
</tags>
<!-- ... -->
</parameters>
> Applying upgrades to tagged units:
"Tags can be added to the upgrade XML's existing <effects> element. They can be mixed with existing <unit> elements. The upgrade will thus be applied to all units in the player's faction which have that tag."
<upgrade>
<!-- ... -->
<effects>
<unit name="roundtent"/>
<tag name="test" />
<unit name="archer"/>
<tag name="warrior" />
</effects>
<!-- ... -->
</upgrade>
> Applying attack boosts to tagged units:
"Tags can also be added to the target of attack boosts. The syntax is similar to that of upgrades. The target value (foe, ally, all, etc) will be obeyed."
<skill>
<!-- ... -->
<attack-boost>
<allow-multiple-boosts value="false" />
<radius value="5" />
<target value="all" include-self="false">
<unit-type name="archer" />
<tag name="warrior" />
</target>
<!-- ... -->
</attack-boost>
<!-- ... -->
</skill>
> Translating tags
Placed in the .lang file of the techtree. Has the source code been already implemented?
TagName_warrior=Guerrier
-
Can you establish that all this works in Megaglest 3.12.0 ?
Do you have an idea where this should be added in the wiki ? ( https://docs.megaglest.org/XMLs)
-
Tags seem to work fine 3.12.0 :)