Exposing more terrain+unit information to WML
Moderator: Forum Moderators
-
- Inactive Developer
- Posts: 787
- Joined: March 31st, 2006, 6:55 am
Re: WML: not pushing units to terrain they are unable to ent
I know it was a proof-of-concept hack, that's why I wanted to point you to a better solution.viorc wrote:Yeah, I would say the lineSkeletonCrew wrote:The problem with this code is that this part "underlying_mvt_terrain(loc_terrain[0]);" gets the movement alias of the terrain. Most of the time it's a single letter but sometimes a multiple characters which can be prefixed by a + or -, which is an invalid terrain. This means you have to parse the string. The real underlying terrain differs per unit. unit->unit_->movement_cost(map.get_terrain(*loc->location_)) does this for you.was powerfully buggy. But as I said it was a hack, more like a kind of proof of concept.Code: Select all
loc_terrain_type = get_terrain_info(loc_terrain_mvt_type[0]);
This function returns at the moment strings like "-Sf" but that can change in the future, in fact I'm working on that part of the source, but not sure whether it's going to change.viorc wrote:My lack of knowledge of fundamentals in terrain handling prevents me from understanding this point. But I happily trust you.SkeletonCrew wrote:The other problem with the direct access to underlying_mvt_terrain() is, when a developer changes the underlying system it will break the WML. With the indirect access the developer will also change unit->unit_->movement_cost() so your WML doesn't need to care about the changes.
If I read this correctly I think I haven't explained good enough what movement_cost() does. This function determines the movement cost on a certain unit on a certain terrain type. Note if the unit is slowed the cost is doubled so you should disable the slowed status of the unit (a bit hacky) or write another C++ function. The output of this function is also seen in Wesnoth if you select a unit and move the mouse over certain locations.viorc wrote:So where do you think I should go from here ? Use the movement_cost() function obviously.
But if done in [teleport] function I think is still not good. For example, in some campaign, teleport is used to move units on the map for story purpose. For example, in A New Order, in scenario "She-wolf of Haeltin", a 6 MP unit is moved from (10,7) to (8,18 ) with teleport. A check that this movement is possible would break it.
Could we meet such kind of problem if put into [unstore_unit] ?
Or again modifying the [store_locations] tag. This time is becomes more tricky as in the call to this function, we have no garrantee to have a unit as input (after reading the function movement_cost(), I see that without unit,it is impossible to know what terrain an alias terrain is aliassing to as it depend on the type of the unit itself).
Create a new [store_...] function [requiring a unit as input] ?
I would call it movement_cost_internal It recurses via movement_cost() so adding a function which calculates the normal movement is a bit a hassle at the moment. I haven't figured out why it recureses via movement_cost() but I'm tempted to remove it in my expirimental branch and see what breaks.viorc wrote:
Could we move the logic of de-aliassing terrain from unit::movement_cost() to a new unit::underlying_terrain() ?
Re: WML: not pushing units to terrain they are unable to ent
My idea is more a function movement_cost_internal_internal() which's code would be something like (another quick hack, take care):SkeletonCrew wrote:I would call it movement_cost_internal It recurses via movement_cost() so adding a function which calculates the normal movement is a bit a hassle at the moment. I haven't figured out why it recureses via movement_cost() but I'm tempted to remove it in my expirimental branch and see what breaks.viorc wrote:
Could we move the logic of de-aliassing terrain from unit::movement_cost() to a new unit::underlying_terrain() ?
Code: Select all
gamemap::TERRAIN unit::movement_cost_internal_internal(gamemap::TERRAIN terrain, int recurse_count) const
{
const int impassable = 10000000;
gamemap::TERRAIN res = terrain;
wassert(map_ != NULL);
//if this is an alias, then select the best of all underlying terrains
const std::string& underlying = map_->underlying_mvt_terrain(terrain);
if(underlying.size() != 1 || underlying[0] != terrain) {
bool revert = (underlying[0] == '-'?true:false);
if(recurse_count >= 100) {
return gamemap::VOID_TERRAIN;
}
int best_value = revert?0:impassable;
for(std::string::const_iterator i = underlying.begin(); i != underlying.end(); ++i) {
if(*i == '+') {
revert = false;
continue;
} else if(*i == '-') {
revert = true;
continue;
}
const int value = movement_cost_internal_internal(*i,recurse_count+1);
if(value < best_value && !revert) {
best_value = value;
res = *i;
} else if(value > best_value && revert) {
best_value = value;
res = *i;
}
}
}
return res;
}
With basic terrain I mean one for which a movement cost is known from the unit stats.
Why not make a new WML tag that takes a unit and an x,y location and sets a movement-cost variable and/or defense variable? That's what you really need, and it would keep the implementation details encapsulated from the WML, and easily accessible by the WML.
http://www.wesnoth.org/wiki/User:Sapient... "Looks like your skills saved us again. Uh, well at least, they saved Soarin's apple pie."
-
- Inactive Developer
- Posts: 787
- Joined: March 31st, 2006, 6:55 am
Tha's the idea yes, the question is how to code itSapient wrote:Why not make a new WML tag that takes a unit and an x,y location and sets a movement-cost variable and/or defense variable? That's what you really need, and it would keep the implementation details encapsulated from the WML, and easily accessible by the WML.
The 'problem' is unit::movement_cost also calculates the poison at the moment, so I would be nice if that could be ignored. The other 'problem' is unit::movement_cost_internal recurses via unit::movement_cost. The solution would be to make unit::movement_cost_internal recurse itself and add and optional parameter to unit::movement_cost to calculate it for the normal case. (Add taking stoned in account would be nice *hint*).
IIRC the recursion change is trivial and shouldn't break anything and the other change is also trivial.
@viorc, I'm not really fond of code duplication if it's not required. In this case I think there's a much better way to achieve the goal, so a tested patch doing that would be nice
Probably a misunderstanding as I do not plan any code duplication (as said current movement_cost_internal() would call new movement_cost_internal_internal(), the later replacing thus most of the code in the former).SkeletonCrew wrote:@viorc, I'm not really fond of code duplication if it's not required. In this case I think there's a much better way to achieve the goal, so a tested patch doing that would be nice
For the tested patch, I can test it but I want to make sure first it goes in the right direction. I would not like to spend time testing a patch that would be rejected by design
Yeah, maybe something like that would be good. Something like...this:Sapient wrote:Why not make a new WML tag that takes a unit and an x,y location and sets a movement-cost variable and/or defense variable? That's what you really need, and it would keep the implementation details encapsulated from the WML, and easily accessible by the WML.
Code: Select all
[store_location_details]
[filter]
description=Jane
[/filter]
x,y=10,12
variable=foo
[/store_location_details]
Code: Select all
[foo]
x=10
y=12
id=forest
name="Forest"
char=f
movement_cost=2
defense=60
[/foo]
There is no point in combining location details and unit-specific terrain details in the same tag. The two functions should be separate.
http://www.wesnoth.org/wiki/User:Sapient... "Looks like your skills saved us again. Uh, well at least, they saved Soarin's apple pie."
They could be split into two, sure. However, in that case I'd rather move the extra terrain details into [store_locations] (and add a key to it that defines what details are stored, to make WML tasks presumably a bit faster). Or they could even all be bundled into [store_locations]. I like having a smaller number of versatile tags and constructs, instead of a dozen specialized ones.Sapient wrote:There is no point in combining location details and unit-specific terrain details in the same tag. The two functions should be separate.
-
- Inactive Developer
- Posts: 787
- Joined: March 31st, 2006, 6:55 am
@viorc what I meant is the following patch (untested)
Then you can change unit::movement_cost() to ignore posion and you have a function which can determine the movement cost for a unit on a certain terrain. I don't have time to test the patch, but I'm quite sure it works
Code: Select all
Index: src/unit.cpp
===================================================================
--- src/unit.cpp (revision 14028)
+++ src/unit.cpp (working copy)
@@ -2139,7 +2139,7 @@
revert = true;
continue;
}
- const int value = movement_cost(*i,recurse_count+1);
+ const int value = movement_cost_internal(*i,recurse_count+1);
if(value < ret_value && !revert) {
ret_value = value;
} else if(value > ret_value && revert) {
I assume by poison you mean 'slowed.' Being slowed essentially doubles the movement cost-- it seems like a useful behavior to me, so I don't know why you are trying to work around it.
http://www.wesnoth.org/wiki/User:Sapient... "Looks like your skills saved us again. Uh, well at least, they saved Soarin's apple pie."
-
- Inactive Developer
- Posts: 787
- Joined: March 31st, 2006, 6:55 am
yes I mean slowedSapient wrote:I assume by poison you mean 'slowed.' Being slowed essentially doubles the movement cost-- it seems like a useful behavior to me, so I don't know why you are trying to work around it.
The reason why you want to get around slowed is since viorc wants to know whether a unit can move on a certain terrain. If not a unit shouldn't be knocked on that terrain. If you ignore slowed a slowed unit has some advantages. Also the recursion seems odd to me I'm even considering to change it anyways.
But what if it's a new unit type that is always 'slowed'? Then it really can't move onto the terrain ... this knockback ability seems like a special case-- if it wants to ignore slowed movement, it should set the unit to not be slowed, then check the function result, IMO.SkeletonCrew wrote:yes I mean slowedSapient wrote:I assume by poison you mean 'slowed.' Being slowed essentially doubles the movement cost-- it seems like a useful behavior to me, so I don't know why you are trying to work around it.
The reason why you want to get around slowed is since viorc wants to know whether a unit can move on a certain terrain. If not a unit shouldn't be knocked on that terrain. If you ignore slowed a slowed unit has some advantages. Also the recursion seems odd to me I'm even considering to change it anyways.
http://www.wesnoth.org/wiki/User:Sapient... "Looks like your skills saved us again. Uh, well at least, they saved Soarin's apple pie."
That is why I would like to have access to the underlying terrain only (through the [store_locations] function) as the movement cost and defense can be retrieved from the [store_unit] tag if we got the value of the basic terrain underlying the location.zookeeper wrote:They could be split into two, sure. However, in that case I'd rather move the extra terrain details into [store_locations] (and add a key to it that defines what details are stored, to make WML tasks presumably a bit faster). Or they could even all be bundled into [store_locations]. I like having a smaller number of versatile tags and constructs, instead of a dozen specialized ones.Sapient wrote:There is no point in combining location details and unit-specific terrain details in the same tag. The two functions should be separate.
The 'underlying terrain' depends on whether you are interested in DEF or movement, it requires handling special cases (such as 'slowed') and aliases, and it involves assumptions about the unit structure which may change between versions. IMO, this function deserves a C++ implementation.
http://www.wesnoth.org/wiki/User:Sapient... "Looks like your skills saved us again. Uh, well at least, they saved Soarin's apple pie."
-
- Inactive Developer
- Posts: 787
- Joined: March 31st, 2006, 6:55 am
I said the same before and yes I agree. I don't even know whether WML has the string manipulation functions to extract the proper data. And the internal terrain structure *is* being changed at the moment.Sapient wrote:The 'underlying terrain' depends on whether you are interested in DEF or movement, it requires handling special cases (such as 'slowed') and aliases, and it involves assumptions about the unit structure which may change between versions. IMO, this function deserves a C++ implementation.