[event] name=sighted behavior

[lotsofphil]

I am trying to understand the name=sighted event. The code I am looking at is TEI scenario 14.

Code: Select all

    [event]
        name=sighted
        [filter]
            id=Khrakrahs
        [/filter]
        [redraw]
            side=1
        [/redraw]
        [scroll_to]
            x,y=$x1,$y1
        [/scroll_to]
        [message]
            caption= _ "Dacyn"
            image=portraits/dacyn.png
            message= _ "Aha! I see what they have done. They have raised him as an undead dragon. But he does not appear to be weak to my arcane senses... this is strange magic indeed."
        [/message]
    [/event]

According to the wiki, this should fire when I move a unit and find Khrakrahs *or* when Khrakhras moves and sees one of my units.

Code: Select all

Triggers when the primary unit becomes visible to the secondary unit in particular after not being visible to the secondary unit's side (so if the secondary unit's side doesn't have shroud or fog, the event never triggers). This happens both when the primary unit moves into view during its turn, and when the secondary unit moves to a location where it can see the primary unit. (This editor hasn't tested whether the event triggers multiple times if the primary unit moves into view of multiple units at once, or if not, which one gets chosen to be the secondary unit here.) (Note: it appears that when a sighted event is triggered because an enemy unit moves into your field of view, the game engine cannot determine which unit (on your side) sees the unit that moved, and so it fires a name=sighted event without setting $second_unit. This means that, for example, using speaker=second_unit inside a message tag may fail.) 

Is that right? I am asking because I played this, Khrakhras moved into my view and the event wasn't triggered. Khrakrahs re-entered the shroud, I found him and then the event triggered.

I am trying to understand the tag and then fix the code if need be.
Windows XP, 1.7.5.

[Sapient]

This is one of the many problems with the sighted event.
It has a lot of bugs and we even considered removing it due to all the problems.
Until it is fixed or replaced with an alternative, I suggest you try this instead:

Code: Select all

[event]
  name=side turn,moveto,attack
  first_time_only=no
  [if]
    [have_unit]
      id=Khrakrahs
      [filter_vision]
      [/filter_vision]
    [/have_unit]
    [variable]
      name=spotted
      not_equals=true
    [/variable]
   [then]
      {VARIABLE spotted true}

# do stuff here

   [/then]
   [else]
     {ALLOW_UNDO}
   [/else]
  [/if]
[/event]

[lotsofphil]

Okay. I will try that out. Thanks.

[solsword]

I've updated the wiki a bit to warn about this. The text that you quoted was accurate at one point during 1.5.x development... but obviously is now incorrect. If someone wants to put something more graceful on the wiki, that would probably be good, but for now I just put up a note saying "this may be wrong."

Also, from the looks of your original code, it seems like you want to fire the event when an enemy unit moves into vision. In that case, you'll probably want a "viewing_side=" in your [filter_vision] tag. Also, I think that just using "moveto" as the event name should be sufficient (although you should test this, obviously). Of course, the one big weakness of "moveto" as a replacement for "sighted" is that it doesn't fire when your unit moves and reveals the enemy. Lukcily, "sighted" still works well enough to do that (other bugs with sighted notwithstanding) so it's possible to do something like this:

Code: Select all

[event]
    name=moveto
    [filter]
        id=Khrakrahs
        [filter_vision]
            viewing_side=1 # Assuming that the player is side 1 here...
        [/filter_vision]
    [/filter]
    [fire_event]
        name=Khrakrahs_spotted
    [/fire_event]
[/event]

[event]
    name=sighted
    [filter]
        id=Khrakrahs
    [/filter]
    [fire_event]
        name=Khrakrahs_spotted
    [/fire_event]
[/event]

Then you can just write the Khrakrahs_spotted event and put whatever you like in there. Note that this way, first_time_only=yes (the default) is your friend (i.e. the Khrakrahs_spotted event can't be triggered twice), but you'd actually have problems with Sapient's code unless you set first_time_only=no in that event.

[Sapient]

ah, good catch on the first_time_only thing.
I updated my post accordingly.

This would probably be a good candidate for a macro...

like:

Code: Select all

{BEGIN_SIGHTED (race=dwarf) 1}
  [message]
    id=$unit.id
    message="I see a midget"
  [/message]
  [message]
    id=$second_unit.id
    message="no, I'm a dwarf"
  [/message]
{END_SIGHTED}

[solsword]

Although the macro solution does fix 90% of the problem cases, I'd rather see the name=sighted event fixed in the code than watch the old functionality get patched up with a macro: I'm all for using that macro in UMC, but I feel that putting it into the mainline macros would discourage people from fixing name=sighted events, which would be bad? Of course, if no-one is going to fix sighted any time soon, perhaps it should officially be limited to units sighted on your turn, and people can use moveto to make up for most of the other claimed functionality?

[Anonymissimus]

Of course, the one big weakness of "moveto" as a replacement for "sighted" is that it doesn't fire when your unit moves and reveals the enemy. Lukcily, "sighted" still works well enough to do that (other bugs with sighted notwithstanding) so it's possible to do something like this:

Code: Select all

[event]
    name=moveto
    [filter]
        id=Khrakrahs
        [filter_vision]
            viewing_side=1 # Assuming that the player is side 1 here...
        [/filter_vision]
    [/filter]
    [fire_event]
        name=Khrakrahs_spotted
    [/fire_event]
[/event]

[event]
    name=sighted
    [filter]
        id=Khrakrahs
    [/filter]
    [fire_event]
        name=Khrakrahs_spotted
    [/fire_event]
[/event]

Then you can just write the Khrakrahs_spotted event and put whatever you like in there. Note that this way, first_time_only=yes (the default) is your friend (i.e. the Khrakrahs_spotted event can't be triggered twice),...

What about the variables unit and second_unit in Khrakrahs_spotted event?

-Khrakrahs_spotted fired from the first event:
unit=Khrakrahs, second_unit=empty ?
What about several units in whose visibility ranges Khrakrahs is after his move...

-Khrakrahs_spotted fired from the second event:
unit=Khrakrahs, second_unit=the unit that revealed Khrakrahs ?
In this specific case, the choices for both variables are rather easy, that's probably why sighted is coping well with it .

But what's unit in the case, that the unit to be spotted was no specific one

Code: Select all

[event]
    name=sighted
    [filter]
        side=2
    [/filter]
    [fire_event]
        name=spotted
    [/fire_event]
[/event]

and there are more than one possible choices for unit ?
What about the case, that delay shroud is on and shroud update after moving reveals unit(s) to be spotted (in which case only the sighted event fires immediately, and in which case there can be several possible choices for unit as well as for second_unit) ?

The more I'm trying to code a workaround for this sighted event problem the more infinitely complicated it gets...

[lotsofphil]

Thanks to both of you. The fix works and I made a patch and submitted it with a bug report (so someone commits it, hopefully).
https://gna.org/bugs/index.php?14374

[solsword]

The more I'm trying to code a workaround for this sighted event problem the more infinitely complicated it gets...

You're absolutely right. That's probably the reason that sighted doesn't work right now: it's a very complicated problem. But it's not as if those questions don't have solutions. Someone just needs to decide on a rule (even an arbitrary rule, like "whichever one the code happens to touch on first" (which might in-practice translate to "the most northern and western of the candidates") and implement it, either in code, or in WML (which would be slow and probably more painful). Of course, there's also the question of how much effort the coder puts in to get a good list of candidates... but really, we can work with whatever arbitrary decisions are made about how to resolve ambiguous sighted cases. What we can't work with is a sighted event that doesn't fire at all in those cases. But I'd like to stress that I'm not really trying to demand that this be fixed (if it sounded like that, I'm sorry): I recognize that it's a complicated problem, so to get fixed, it needs someone willing to put a bit of effort into it. And since there's a workaround that covers *most* of the cases, there's no real pressure to fix it urgently. I was just pointing out that if the workaround becomes a standard macro, it's less likely that someone will *eventually* fix this, and the result will be that the other 10% of cases will be abandoned.

In terms of $unit and $second_unit issues, the code as I wrote it doesn't have any values in the fired event, as far as I understand it. To get those, you'd have to add [primary_unit] and [secondary_unit] tags to the [fire_event] tag. As you point out, these should be something like "id=$unit.id" for the second case, and for the first, you might drop the [secondary_unit] altogether. Of course, you could do something where you searched for the closest unit on the viewing side (I've got a macro for doing a search for a closest hex that meets arbitrary conditions in my campaign... but it's an absolutely awful algorithm and terribly slow).

[Anonymissimus]

I have finally a solution that I'm rather confident with. I avoids the name=sighted event completely, since to my experience, this doesn't reliably fire, even in case 2: unit(s) to be spotted become visible during the player's moves.
According to my tests so far it works now even better than I would have expected, after trying a ton of different approaches. There's now some testing by others and comments or further improvements needed.
I wrote the macro in particular for cave scenarios with shroud and no fog, but I'm guessing it should work with fog, too (replace DK_REMOVE_SHROUD macro with CLEAR_FOG then)
It uses the NEAREST_HEXT macro from Dead Water (that macro is well-coded and as efficient as possible, I put it into my campaign, thx beetlenaut).

Code: Select all

#That's the nearest hex macro taken "as is" from Dead Water - it matches exactly the thing that I need at the moment.
# Finds the nearest hex to (X,Y) within MAX_DISTANCE that matches the FILTER, and stores the location in VARIABLE with x, y, and terrain. If there is more than one location found, VARIABLE will be an array.
#define NEAREST_HEX X Y MAX_DISTANCE FILTER VARIABLE
	[set_variable]
		name=distance
		value=0
	[/set_variable]

	[set_variable]
		name=location_found
		value=no
	[/set_variable]

	# Store any matching locations within a radius of "distance". If nothing got stored, add one to "distance" and try again. Do this until a location is found, or "distance" goes over the MAX_DISTANCE.
	[while]
		[variable]
			name=location_found
			equals=no
		[/variable]
		[and]
			[variable]
				name=distance
				less_than_equal_to={MAX_DISTANCE}
			[/variable]
		[/and]

		[do]
			[store_locations]
				{FILTER}
				[and]
					x={X}
					y={Y}
					radius=$distance
				[/and]
				variable={VARIABLE}
			[/store_locations]

			[if]
				[variable]
					name={VARIABLE}.length
					greater_than=0
				[/variable]
				[then]
					[set_variable]
						name=location_found
						value=yes
					[/set_variable]
				[/then]
			[/if]

			[set_variable]
				name=distance
				add=1
			[/set_variable]
		[/do]
	[/while]

	[clear_variable]
		name=distance
	[/clear_variable]

	[clear_variable]
		name=location_found
	[/clear_variable]
#enddef

----------------------------------------

Code: Select all

#define SIGHTED_MOVETO FILTER VIEWING_SIDE RADIUS EVENT_NAME IMPASSABLE WML
	# 	purpose of this macro: replacing a name=sighted event while
	# 	-reliably firing on all cases a sighted event should have
	# 	-reliably providing sensible values for $unit and $second_unit
	# 	-any standard unit filter keys should work to define the unit(s) to be spotted
	# 	-the buggy name=sighted event is completely avoided, this works only with moveto, filtering and conditional wml

	# 	disadvantages:
	# 	-delay shroud is ignored; if the player would have revealed unit(s) to be spotted, the event is fired and shroud updated
	# 	-the event sometimes fires although sighted shouldn't have - if there is terrain slowing the viewing units down

	# 	usage:
	# 	FILTER=filter on unit(s) to be spotted
	# 	VIEWING_SIDE=side number of viewing side
	# 	RADIUS=should match the map size; maybe that key can be left away
	# 	EVENT_NAME=you can provide a unique name for the sighted event in order to have several sighted events in the same scenario
	# 	IMPASSABLE=list of terrain strings that are impasable for the viweing units in this scenario
	# 	WML=wml to be executed in the sighted event
	
	# 	example:
	# 	{SIGHTED_MOVETO side=2 1 20 troll_sighted "Xu,Qxu,Ql" (
	# 	[message]
	# 		speaker=second_unit
	# 		message=_"I see a troll!"
	# 	[/message]
	# 	[message]
	# 		speaker=unit
	# 		message=_"I see a dwarf!"
	# 	[/message]
	# 	)}
	#---------------------------------------------------
	
	#case 1: unit(s) to be spotted enter the viewing side's view
	[event]
		#this event triggers if the unit to spot moves into the player's visibility range
		#delay shroud does not interfere here, since it's not the player's turn so he can't delay his shroud update
		#at the end of the player's turn, his shroud had been updated automatically

		name=moveto
		[filter]
			{FILTER}
			[filter_vision]
				viewing_side={VIEWING_SIDE}
			[/filter_vision]
		[/filter]
		#{DEBUG_MSG 1}

		#choosing a suitable second unit is difficult, since there can be more than one
		#take the first one from the array loc_SecondUnit, if there are more than one in the same distance
		#NEAREST_HEX macro can be found in Dead Water
		#it calculates here for unit A at $x1 $y1 the position of unit B on VIEWING_SIDE that's at max RADIUS away but as near as possible
		{NEAREST_HEX $x1 $y1 {RADIUS} (
		[filter]
			side={VIEWING_SIDE}
		[/filter]
		) loc_SecondUnit}

		[fire_event]
			name={EVENT_NAME}
			[primary_unit]
				x=$x1
				y=$y1
			[/primary_unit]
			[secondary_unit]
				x=$loc_SecondUnit.x
				y=$loc_SecondUnit.y
			[/secondary_unit]
		[/fire_event]
	[/event]
	
	#---------------------------------------------------
	#case 2: unit(s) of the viewing side move, making unit(s) to be spotted visible
	[event]
		name=moveto #avoiding name=sighted completely, since it also doesn't fire reliably in this case
		first_time_only=no
		[filter]
			#filter vision doesn't allow filtering on specific units
			side={VIEWING_SIDE}
		[/filter]
		#{DEBUG_MSG 2}
		[if]
			#variable query that prevents unneccessary storing of units
			#if the sighted event hasn't already been executed, test wether unit(s) to be spotted are visible, else allow undo
			[variable]
				name=b_{EVENT_NAME}
				boolean_not_equals=yes
			[/variable]
			[then]
				#add one to the movement range since this seems the normal behaviour of the game engine to determine visibility range
				{VARIABLE i_VisibilityRadius "$($unit.max_moves+1)"}
				#{DEBUG_MSG $i_VisibilityRadius}
				
				#store spotted unit(s) that are to be spotted/"calculate the viewing area"
				#It's remarkable that the radius does not only extend at straight lines outwards to the edge of the circular region,
				#but also around corners, representing the visibility area exactly in case there are no other movement limitations.
				[store_unit]
					variable=u_Unit #becomes the sighted event's primary unit
					[filter]
						{FILTER}
						[filter_location]
							x=$x1
							y=$y1
							[filter_radius]
								[not]
									terrain={IMPASSABLE}
								[/not]
							[/filter_radius]
							radius=$i_VisibilityRadius
						[/filter_location]
					[/filter]
				[/store_unit]
# 				[store_locations]
# 					variable=test
# 					x=$x1
# 					y=$y1
# 					[filter_radius]
# 						[not]
# 							terrain={IMPASSABLE}
# 						[/not]
# 					[/filter_radius]
# 					radius=$i_VisibilityRadius
# 				[/store_locations]
# 				{FOREACH test i}
# 					#{DEBUG_MSG "$i|: $test[$i].x|,$test[$i].y|"}
# 					{SET_LABEL $test[$i].x $test[$i].y _"$i"}
# 					{NEXT i}
				
				#{DEBUG_MSG $u_Unit.id}
				
				#query wether a unit has been spotted, then execute the event, else allow undo
				[if]
					[variable]
						name=u_Unit.length
						greater_than=0
					[/variable]
					[then]
						#In case that the player has delay shroud on, make the spotted unit visible
						#macro removes shroud on a circular region around $u_Unit.x,$u_Unit.y with radius 1
						#can be found in my campaign
 						{DK_REMOVE_SHROUD $u_Unit.x $u_Unit.y 1}
						
						[fire_event]
							name={EVENT_NAME}
							[primary_unit]
								x=$u_Unit.x
								y=$u_Unit.y
							[/primary_unit]
							[secondary_unit]
								x=$x1
								y=$y1
							[/secondary_unit]
						[/fire_event]
					[/then]
					[else]
						[allow_undo]
						[/allow_undo]
					[/else]
				[/if]
			[/then]
			[else]
				[allow_undo]
				[/allow_undo]
			[/else]
		[/if]
	[/event]
	[event]
		name={EVENT_NAME}
		{VARIABLE b_{EVENT_NAME} yes}
		
		#{DEBUG_MSG $unit.id}
		#{DEBUG_MSG $second_unit.id}
		
		{WML}
	[/event]
	[event]
		name=victory
		{CLEAR_VARIABLE i_VisibilityRadius}
		{CLEAR_VARIABLE loc_SecondUnit}
		{CLEAR_VARIABLE b_{EVENT_NAME}}
		{CLEAR_VARIABLE u_Unit}
	[/event]
#enddef

[solsword]

While this is a good solution, it has several disadvantages (some of which you mentioned) which make it problematic in certain situations. The big disadvantage that you didn't mention is that it doesn't really work for (and you can't easily make a version that works for) first_time_only=no. Making a macro to, for example, count the number of enemy units a player had seen, wouldn't work with this (of course, with a working version of name=sighted, there'd be the problem of repeated sighting events, but you could at least get a good estimate). Of course there are more tricks that you can add, but a working name=sighted event wouldn't necessitate those. The other main disadvantage (which you mentioned) is that this doesn't take into account terrain that slows movement. This may seem like not a big deal, but with certain units, like merfolk and cavalry, a lot of terrain is terrain that slows movement. To me, at least, this shortcoming is nothing to ignore. Since you seem to have experimented with this: under what circumstances doesn't the sighted event fire properly for units moved during your turn? Is it something to do with delay_shroud? Or something else? I haven't really done any testing for edge cases, but in my experience name=sighted events have fired just fine for units spotted during your turn. If that's not the case, I need to revise some of my macros (I'll probably use your solution), but if it is the case, then using name=sighted instead of moveto for the second case in your macro would eliminate the poor handling of restrictive terrains.

The last potential problem is how slow this macro could be. From what I can see, it's using basically the same algorithm that I am: evaluate a terrain filter at an ever-increasing radius, until you find what you're looking for. In my experience, using this frequently adds a slight but noticeable slowdown to the game. Since your vision macro involves running this expensive operation on every allied moveto event, it might be a nuisance to those with slower hardware.

For reference, the search filter that I came up with is almost exactly the same as yours (I don't remember whether I looked at Dead Water's filter or not, but I think I did). One way to improve it is to tie the [while] loop directly to {VARIABLE}.length: that way you don't need the extra location_found variable and you can get rid of the whole if/else statement within the while loop. In my macro, I use preset variable names, just like the RANDOM macro, to avoid having to pass in a variable name, and I don't clear the distance variable, which means that the macro can be used to measure distances as well. Also note that you can clear two variables in a single [clear_variables] tag using name=a,b (where 'a' and 'b' are the variables to clear). In addition, you should almost certainly clear {VARIABLE} at the beginning of the macro, otherwise things would break down if you called the macro twice with the same value of {VARIABLE}.

The full code for my version of the search macro (plus a cleanup macro):

Code: Select all

#define FIND_NEARBY FILTER X Y LIMIT
    # Does a search for a nearby location that matches the given filter.
    # Basically just looks for such a location with increasing radius until it
    # finds at least one. This is sadly inefficient, but implementing BFS in
    # WML is... difficult. Once LIMIT is reached, the entire map is searched.
    # This macro creates the nearby_locations and nearby_distance variables,
    # which can be used to access a list of locations found and the distance to
    # those locations, respectively. They should eventually be cleared, which
    # can be accomplished using the CLEANUP_SEARCH macro.
    [clear_variable]
        name=nearby_locations
    [/clear_variable]
    [set_variable]
        name=nearby_distance
        value=0
    [/set_variable]
    [while]
        [variable]
            name=nearby_locations.length
            equals=0
        [/variable]
        [and]
            [variable]
                name=nearby_distance
                less_than={LIMIT}
            [/variable]
        [/and]
        [do]
            [store_locations]
                variable=nearby_locations
                {FILTER}
                [and]
                    x,y={X},{Y}
                    radius=$nearby_distance
                [/and]
            [/store_locations]
            [set_variable]
                name=nearby_distance
                add=1
            [/set_variable]
        [/do]
    [/while]
    [if]
        [variable]
            name=nearby_locations.length
            equals=0
        [/variable]
        [then]
            [store_locations]
                variable=nearby_locations
                {FILTER}
            [/store_locations]
        [/then]
    [/if]
#enddef

#define CLEANUP_SEARCH
    # Clears variables involved in searching (the FIND_NEARBY macro). Put this
    # in your name=victory,defeat tag to clean up if you use FIND_NEARBY within
    # a scenario.
    [clear_variable]
        name=nearby_locations, nearby_distance
    [/clear_variable]
#enddef

[Anonymissimus]

The big disadvantage that you didn't mention is that it doesn't really work for (and you can't easily make a version that works for) first_time_only=no.

Right.
That just didn't belong to the number of tasks for the macro which I had in mind when making it. Although I suppose with some improvements based on my macro - inserting first_time_omly=no everywhere, introducing variables to control what is executed at what time - it should be possible. That's up to you then or someone else.

The other main disadvantage (which you mentioned) is that this doesn't take into account terrain that slows movement. This may seem like not a big deal, but with certain units, like merfolk and cavalry, a lot ofterrain is terrain that slows movement.

Well, I rather have units in my campaign that are slowed by rather few terrain types. Also, the advantage of a reliably firing event is more worthy for me than the disadvantage that it possibly fires too early - remember that the sighted event would (should) have fired just one or few turns later.

Since you seem to have experimented with this:

Not with name=sighted. I've avoided it also in tests because of the generally buggy behaviour.

under what circumstances doesn't the sighted event fire properly for units moved during your turn? Is it something to do with delay_shroud?

Apparently. Played a scenario recently with the impression that there must've been a sighted event that didn't fire. Browsing the code proved this. I was using delay shroud in that scenario.
The missing event execution changed the scenario completely. This can even cause severe bugs, or make a scenario unable to beat. So better put only expendable dialog in such events...

The last potential problem is how slow this macro could be. From what I can see, it's using basically the same algorithm that I am: evaluate a terrain filter at an ever-increasing radius, until you find what you're looking for. In my experience, using this frequently adds a slight but noticeable slowdown to the game. Since your vision macro involves running this expensive operation on every allied moveto event, it might be a nuisance to those with slower hardware.

I have slow hardware and the slowdown is still acceptable I think. I hardly noticed anything when testing the macro.

One way to improve it is to tie the [while] loop directly to {VARIABLE}.length: that way you don't need the extra location_found variable and you can get rid of the whole if/else statement within the while loop.

Right. I'll improve the algorythm.

In my macro, I use preset variable names, just like the RANDOM macro, to avoid having to pass in a variable name

I don't understand this or which advantage it may mean.

Also note that you can clear two variables in a single [clear_variables] tag using name=a,b (where 'a' and 'b' are the variables to clear).

I treat variables usually as "scenario variables" which are only cleared once in the victory event. Using CLEAR_VARIABLE seperately makes the code more readable then. I'll remember it if there's a loop with more than one local variable to be cleared.

In addition, you should almost certainly clear {VARIABLE} at the beginning of the macro, otherwise things would break down if you called the macro twice with the same value of {VARIABLE}.

The coder who's using my macro is expected to call it with different EVENT_NAMEs every time, thus the variable names will be different. Otherwise, there would be several events with the same name anyway, resulting in strange behaviour. For sighted event A the WML entered in A and B would be executed, the same for sighted event B, and each time in unknown order, I suppose...

Finally, there's a possible improvement in your macro: Using numerical_equals instead of equals improves performance, the same for boolean_equals - if you have these types of variables. I think I've read it somewhere...

/edit
Found a potential problem in your macro. Before querying the length, you should probably initialize the array, about

Code: Select all

	[store_locations]
		variable=...
		[not]
		[/not]
	[/store_locations]

[solsword]

Apparently. Played a scenario recently with the impression that there must've been a sighted event that didn't fire. Browsing the code proved this. I was using delay shroud in that scenario.
The missing event execution changed the scenario completely. This can even cause severe bugs, or make a scenario unable to beat. So better put only expendable dialog in such events...

This is certainly a major problem, but I was under the impression that it was only buggy during enemy turns (which could have caused the behavior you experienced). In that case, the sighted + moveto combination should be perfectly safe. I just did some very simple testing, and delaying shroud updated didn't seem to affect sighted events on my turn. If it was on, the sighted event fired at the end of my turn, because that's when the shroud cleared, but it still fired. I'm using v1.7.5+svn(r38796). I'll admit that there are some edge cases that I didn't test (like your example of sighting multiple units at once upon shroud update), but at least the basic delay shroud updates doesn't break name=sighted events (I also tested turning it off rather than ending my turn, and that also worked). I think I'll keep using the moveto + sighted combo until I hear about a bug with sighted during the active turn.

I have slow hardware and the slowdown is still acceptable I think. I hardly noticed anything when testing the macro.

Hmmm... maybe I'm calling it a lot more than I should be. I'll have to look at my code now. Or maybe something else is slowing things down...

I don't understand this or which advantage it may mean.

There's no real advantage. I just don't like having to pass it a variable name each time and worrying about whether I'm using different ones in all scenarios. Of course, in return, I have to worry about capturing the variable before I use the macro again, but that generally isn't a huge concern.

I treat variables usually as "scenario variables" which are only cleared once in the victory event. Using CLEAR_VARIABLE seperately makes the code more readable then. I'll remember it if there's a loop with more than one local variable to be cleared.

You're right about this... using two [clear_variable] tags does help make it explicit that you're clearing both and help when you code things to remember to clear all your variables.

Finally, there's a possible improvement in your macro: Using numerical_equals instead of equals improves performance, the same for boolean_equals - if you have these types of variables. I think I've read it somewhere...

Good catch. I remember that too, now that I think about it. I've changed the code to use a [not] combined with greater_than=0. That ought to address the speed issue (I hope?) and the other issue that you brought up (although the code works just fine as I posted it.. kinda strange.

In case anyone copy-pastes form this thread later, here's my updated version of the search code:

code:

Code: Select all

#define FIND_NEARBY FILTER X Y LIMIT
    # Does a search for a nearby location that matches the given filter.
    # Basically just looks for such a location with increasing radius until it
    # finds at least one. This is sadly inefficient, but implementing BFS in
    # WML is... difficult. Once LIMIT is reached, the entire map is searched.
    # This macro creates the nearby_locations and nearby_distance variables,
    # which can be used to access a list of locations found and the distance to
    # those locations, respectively. They should eventually be cleared, which
    # can be accomplished using the CLEANUP_SEARCH macro.
    [clear_variable]
        name=nearby_locations
    [/clear_variable]
    [set_variable]
        name=nearby_distance
        value=0
    [/set_variable]
    [while]
        [not]
            [variable]
                name=nearby_locations.length
                greater_than=0
            [/variable]
        [/not]
        [and]
            [variable]
                name=nearby_distance
                less_than={LIMIT}
            [/variable]
        [/and]
        [do]
            {DEBUG "Searching depth $nearby_distance around ({X}, {Y})..."}
            [store_locations]
                variable=nearby_locations
                {FILTER}
                [and]
                    x,y={X},{Y}
                    radius=$nearby_distance
                [/and]
            [/store_locations]
            {DEBUG "...found $nearby_locations.length locations."}
            [set_variable]
                name=nearby_distance
                add=1
            [/set_variable]
        [/do]
    [/while]
    [if]
        [variable]
            name=nearby_locations.length
            equals=0
        [/variable]
        [then]
            [store_locations]
                variable=nearby_locations
                {FILTER}
            [/store_locations]
        [/then]
    [/if]
#enddef

#define CLEANUP_SEARCH
    # Clears variables involved in searching (the FIND_NEARBY macro). Put this
    # in your name=victory,defeat tag to clean up if you use FIND_NEARBY within
    # a scenario.
    [clear_variable]
        name=nearby_locations, nearby_distance
    [/clear_variable]
#enddef

[silene]

Using numerical_equals instead of equals improves performance, the same for boolean_equals - if you have these types of variables.

No. Using equals is faster. The point of numerical_equals and boolean_equals is not performance, it's representation. For instance, "1" and "1.0" are not equal as strings but they are equal as numbers; and "yes" and "on" are not equal as strings but they are equal as booleans. (This also explains why equals is faster: it is a straightforward comparison that doesn't try to understand what you have written.)

[Anonymissimus]

No. Using equals is faster. The point of numerical_equals and boolean_equals is not performance, it's representation. For instance, "1" and "1.0" are not equal as strings but they are equal as numbers; and "yes" and "on" are not equal as strings but they are equal as booleans. (This also explains why equals is faster: it is a straightforward comparison that doesn't try to understand what you have written.)

That's really bad since I've been using numerical_... and boolean_... on and on...
Then the wiki

Using numerical_equals/numerical_not_equals for numbers results in faster calculation than equals/not_equals.

is wrong. I will put your explanation there instead.
What if variable=1.0 and then variable is queried if it's numerical_equal to 1.0, is that as fast as equals in that case ? But also variable!=1.0 sometimes (otherwise the query wouldn't make sense), resulting in less speed after all...
So numerical_equals or boolean_equals only make sense if the variables' values are inconsistent. But if someone always uses only "no" to represent "false" it's better to use

if
variable
equals="no"
...