[engine] Design of [query_location]

Brainstorm ideas of possible additions to the game. Read this before posting!

Moderators: Forum Moderators, Developers

Forum rules
Before posting a new idea, you must read the following:
User avatar
Ravana
Moderator
Posts: 1688
Joined: January 29th, 2012, 12:49 am
Location: Estonia

Re: [engine] Design of [query_location]

Post by Ravana » January 25th, 2018, 9:40 pm

Current state is that there is adding and deleting mode, and switching and finishing is when adding added one or deleting nonadded one. Nothing happens on hex that is filtered out.

For implementation, since synchronize_choice needs wml table there is type conversion location_set -> wml table -> lua table -> wml variable.

Tad_Carlucci
Developer
Posts: 270
Joined: April 24th, 2016, 4:18 pm

Re: [engine] Design of [query_location]

Post by Tad_Carlucci » January 25th, 2018, 10:57 pm

Ah. I see.

My first reaction is: let it block the game. Open a Feature Request Issue asking that MP Chat be moved to a separate thread. But that's probably not workable without a LOT of work.

Since [options] blocks, I'd still do it like that and try to come up with a way to handle MP Chat which plays nice with both. Most likely, whatever you'd come up with for [query_location] would be virtually identical to what we'd need for [options], anyway. Might as will kill both birds with one stone.
I forked real life and now I'm getting merge conflicts.

User avatar
Sapient
Developer
Posts: 4429
Joined: November 26th, 2005, 7:41 am
Contact:

Re: [engine] Design of [query_location]

Post by Sapient » January 25th, 2018, 11:15 pm

> location filter - unsure what to do when missing

Missing location filter means choose from entire map. Seems fine to me.

> or when it matches no location

This doubtlessly means the programmer goofed up. A warning is in order. Treat it as an automatic cancel.

> whether to fire old callback - or just disallow that if it allows attacking otherwise

This sounds like a Lua implementation detail specific to the way you have chosen to implement the feature; as such, No Comment

> WML variable for result - optional

I think you mean to say that there is a default variable name (“location”?) when unspecified

> custom confirm message - optional

I am very much opposed to having a confirmation pop-up. If the author wants a conformation dialog, that should be an opt-in behavior at most... better yet let them write it themselves since we shouldn’t encourage such annoyances.



> -- custom overlay - optional

I like the idea of being able to define overlays for selectable, unselectable, hover(selectable), and cancel(hover+un selectable).


> WML variable with .x and .y is created

FYI, I dislike the idea of exposing magic values such as -1,-1 to WML on cancel.
The WML variable should be cleared on cancel.
Thus a length check should return zero
AND .x and .y would equals=$empty


> Overlay is added to all matching locations while selecting

Optional, I assume?

> Locations with fog/shroud are undecided how to handle

For fog: Just because you cannot see it doesn’t mean your catapult can’t reach it, right? The author can add a vision filter if need be.
For shroud: could get ugly with overlays maybe, but I guess should be habdled same as For fog

> Cursor change is not possible.

What do you mean “not possible”?

> Not sure how cancel should be reported.

If you mean should there be a warning: No
http://www.wesnoth.org/wiki/User:Sapient... "Looks like your skills saved us again. Uh, well at least, they saved Soarin's apple pie."

User avatar
Ravana
Moderator
Posts: 1688
Joined: January 29th, 2012, 12:49 am
Location: Estonia

Re: [engine] Design of [query_location]

Post by Ravana » January 26th, 2018, 9:46 am

There is no Lua interface to change cursor.

User avatar
Ravana
Moderator
Posts: 1688
Joined: January 29th, 2012, 12:49 am
Location: Estonia

Re: [engine] Design of [query_location]

Post by Ravana » January 30th, 2018, 12:51 pm

Status update. When only one location is asked and custom confirm message is not provided, then no confirm message is displayed.

Fog not handled yet.
I am not content with handling of locations not allowed by filter - currently that finishes selection and based on configuration might display confirm message. And main use case of getting 1 location might not get that then.
I added option to fire old callback, but I am not sure if there is any use case of that.

Current code

Code: Select all

--<<

-- If there is no custom confirm message, and max_count is 1, then confirm message is not used

-- Parameters
-- location filter - unsure what to do when missing or when it matches no location
-- fire_original - whether to fire old callback - or just disallow that if it allows attacking otherwise
-- max_count - optional default 1, how many locations player may select
-- WML variable for result - optional
-- custom confirm message - optional
-- overlay_selected, overlay_selectable - custom overlay - optional
function wesnoth.wml_actions.query_location(cfg)
	local helper = wesnoth.require "lua/helper.lua"
	local overlay_choosable = cfg.overlay_selectable or "terrain/alphamask.png~O(0.2)~CS(255,255,0)"
	local overlay_chosen = cfg.overlay_selected or "terrain/alphamask.png~O(0.2)~CS(0,255,0)"
	local variable = cfg.variable or "location"
	local max_count = cfg.max_count or 1
	local confirm_message = cfg.confirm_message or "Do you want to use chosen locations?"
	local fire_original = cfg.fire_original or false
	local filter = helper.get_child(cfg, "filter_location") or {}
	
	local allowed_locations = wesnoth.get_locations(filter)
	if #allowed_locations == 0 then helper.wml_error("query_location matches no location") end
	
	wesnoth.wml_actions.disallow_end_turn()
	
	local location_set = wesnoth.require "lua/location_set.lua"
	
	local res = wesnoth.synchronize_choice("location", function()
		local old_callback = wesnoth.game_events.on_mouse_action
		local finished = false
		local adding = true
		
		local candidates = location_set.create()
		local chosen_locations = location_set.create()
		for _, loc in ipairs(allowed_locations) do
			x = loc[1]
			y = loc[2]
			candidates:insert(x, y)
			wesnoth.add_tile_overlay(x, y, { image = overlay_choosable })
		end
		wesnoth.redraw{}
		
		function wesnoth.game_events.on_mouse_action(x,y)			
			if adding and candidates:get(x,y) then
				chosen_locations:insert(x,y)
				candidates:remove(x,y)
				wesnoth.remove_tile_overlay(x, y, { image = overlay_choosable })
				wesnoth.add_tile_overlay(x, y, { image = overlay_chosen })
			elseif adding and chosen_locations:get(x,y) then
				finished = true
			elseif not adding and candidates:get(x,y) then
				finished = true
			elseif not adding and chosen_locations:get(x,y) then
				candidates:insert(x,y)
				chosen_locations:remove(x,y)
				wesnoth.remove_tile_overlay(x, y, { image = overlay_chosen })
				wesnoth.add_tile_overlay(x, y, { image = overlay_choosable })
			elseif not candidates:get(x,y) and not chosen_locations:get(x,y) then
				finished = true -- TODO something better to do here?
			end
			
			if chosen_locations:size() >= max_count then
				finished = true
			end

			if fire_original then
				-- do we really want this?
				old_callback(x ,y) -- only if cfg asks for it explicitly
			end
		end

		while not finished do
			while not finished do
				wesnoth.print{text="Click on marked location",size=24}
				wesnoth.delay(10)
			end
			
			if max_count > 1 then
				if chosen_locations:size() < max_count then
					local result = helper.get_user_choice({ speaker = "narrator", message = confirm_message }, { "Yes", "Continue marking", "Toggle mode", "Quit" })
					if result == 4 then
						chosen_locations:iter(function(x,y,data) 
							wesnoth.remove_tile_overlay(x, y, { image = overlay_chosen })
						end)
						chosen_locations:clear()
					elseif result == 3 then
						finished = false
						adding = not adding
					elseif result == 2 then
						finished = false
					end
				else
					local result = helper.get_user_choice({ speaker = "narrator", message = confirm_message }, { "Yes", "Choose again", "Quit" })
					if result == 3 then
						chosen_locations:iter(function(x,y,data) 
							wesnoth.remove_tile_overlay(x, y, { image = overlay_chosen })
						end)
						chosen_locations:clear()
					elseif result == 2 then
						chosen_locations:iter(function(x,y,data) 
							wesnoth.remove_tile_overlay(x, y, { image = overlay_chosen })
							wesnoth.add_tile_overlay(x, y, { image = overlay_choosable })
							candidates:insert(x,y)
						end)
						chosen_locations:clear()
						finished = false
					end
				end
			else
				if cfg.confirm_message then
					local result = helper.get_user_choice({ speaker = "narrator", message = confirm_message }, { "Yes", "Choose again", "Quit" })
					if result == 3 then
						chosen_locations:iter(function(x,y,data) 
							wesnoth.remove_tile_overlay(x, y, { image = overlay_chosen })
						end)
						chosen_locations:clear()
					elseif result == 2 then
						chosen_locations:iter(function(x,y,data) 
							wesnoth.remove_tile_overlay(x, y, { image = overlay_chosen })
							wesnoth.add_tile_overlay(x, y, { image = overlay_choosable })
							candidates:insert(x,y)
						end)
						chosen_locations:clear()
						finished = false
					end
				end
			end
		end
		
		wesnoth.game_events.on_mouse_action = old_callback
		
		candidates:iter(function(x,y,data) 
			wesnoth.remove_tile_overlay(x, y, { image = overlay_choosable })
		end)
		chosen_locations:iter(function(x,y,data) 
			wesnoth.remove_tile_overlay(x, y, { image = overlay_chosen })
		end)
		
		local wml_object = {}
		for i,v in ipairs(chosen_locations:to_pairs()) do
			wml_object[i] = {"value",{x=v[1],y=v[2]}}
		end
		wesnoth.redraw{}
		
		return wml_object
	end)

	local normal_object = {}
	for i=1,#res do 
		normal_object[i] = res[i][2]
	end
	wesnoth.wml_actions.allow_end_turn()
	helper.set_variable_array(variable, normal_object)
	return normal_object
end

-->>
Examples

Code: Select all

[query_location]
[/query_location]
[query_location]
	variable=test_1
[/query_location]
[query_location]
	variable=test_2
	[filter_location]
		[filter]
			canrecruit=yes
		[/filter]
	[/filter_location]
[/query_location]
[query_location]
	variable=test_3
	max_count=4
	[filter_location]
		terrain=Cud
	[/filter_location]
[/query_location]
[query_location]
	variable=test_4
	overlay_selectable="terrain/alphamask.png~O(0.6)~CS(255,0,255)"
	[filter_location]
		terrain=Cud
	[/filter_location]
[/query_location]
[query_location]
	variable=test_5
	confirm_message="Attack there?"
[/query_location]
[query_location]
	variable=test_6
	[filter_location]
		terrain=xyy
	[/filter_location]
[/query_location]

User avatar
Eagle_11
Posts: 701
Joined: November 20th, 2013, 12:20 pm

Re: [engine] Design of [query_location]

Post by Eagle_11 » January 30th, 2018, 3:56 pm

question: can this be used for something like to launch an nuclear missile over an radius of 18 hexes ?

edit: ah, cool. so will this make into 1.14.x maybe ?
Last edited by Eagle_11 on January 30th, 2018, 4:09 pm, edited 1 time in total.

User avatar
Ravana
Moderator
Posts: 1688
Joined: January 29th, 2012, 12:49 am
Location: Estonia

Re: [engine] Design of [query_location]

Post by Ravana » January 30th, 2018, 4:07 pm

Yes, you can still scroll around when selecting.

User avatar
Celtic_Minstrel
Developer
Posts: 1116
Joined: August 3rd, 2012, 11:26 pm
Contact:

Re: [engine] Design of [query_location]

Post by Celtic_Minstrel » February 6th, 2018, 1:08 am

Looks pretty good so far. I would've said don't even think about support for selecting multiple locations; get it working for one location first. It kinda looks like you've gone all out on the multiple locations, though, so maybe the tag should be renamed to the plural.

For multiple locations, I think it should automatically end when you've selected a specified maximum number of locations, but I also think there should be a way to end it early with less than the maximum number of locations. I'm thinking of "press space", but almost anything could work as long as it makes sense. The currently-selected spaces should be hilited (the gold outline would be a good way), and clicking a selected one should deselect it. I think it'd also be good to have a counter somewhere indicating how many spaces you have left, but I'm not sure how that could be done.

There's no need to fire the original callback in my opinion. Just replace it, then restore it when done. What I'd expect to see (for single-location) is actually that the new callback restores the old one when done. (Obviously multiple locations complicates things.)

I don't think a confirmation popup is needed, but it seems like it's a nice option to have, at least. I'm not sure whether it's better to use the message dialog or the confirm dialog (wesnoth.confirm, works the same as the JS function).
Author of The Black Cross of Aleron campaign and Default++ era.
Maintainer of Steelhive.

User avatar
Ravana
Moderator
Posts: 1688
Joined: January 29th, 2012, 12:49 am
Location: Estonia

Re: [engine] Design of [query_location]

Post by Ravana » February 6th, 2018, 1:32 am

When requesting single location then there is no confirm message by default, only when custom one is provided.

Makes sense to finish when there is max amount of locations selected.

I have wesnoth.print while selecting, could include counter there.

Main reason to have confirmation is to get more complex user input than just leftclick on map hex. There is no requirement that filter would invalidate any location, so if clicking selected one would unselect it, then there would be no way to end it early. Again, when requesting one location only, no confirmation needed. From what I know, wesnoth.game_events.on_mouse_action and select event are only reliable ways to get user input in nonblocking way, and that is limiting when there are multiple possible actions - would take click patterns or timing to achieve.

Old callback is restored within wesnoth.synchronize_choice.

User avatar
Celtic_Minstrel
Developer
Posts: 1116
Joined: August 3rd, 2012, 11:26 pm
Contact:

Re: [engine] Design of [query_location]

Post by Celtic_Minstrel » February 6th, 2018, 3:08 am

Ah yes, print is actually a pretty decent way of displaying this information, as that's screen-position overlaid text.

You seem to be implying that a confirmation is required if selecting more than one location. I'd argue this is not the case? I think the confirmation can still be optional when selecting multiple locations.

Unless you want to allow the user to pick fewer than the maximum number of locations, you don't need anything besides wesnoth.game_events.on_mouse_action, so I'm not sure why you're talking about the limits of this method? You'd need a little more to accept fewer than the max though... maybe [query_locations] could call [set_menu_item] to temporarily install a hotkey? I'm not sure if that would work well though...
Author of The Black Cross of Aleron campaign and Default++ era.
Maintainer of Steelhive.

User avatar
Ravana
Moderator
Posts: 1688
Joined: January 29th, 2012, 12:49 am
Location: Estonia

Re: [engine] Design of [query_location]

Post by Ravana » February 8th, 2018, 6:43 pm

Improved this by removing message unless explicitly asked for. I consider it feature-complete at this point.

About comments here so far, - meaning not implemented, ! meaning might change, + meaning implemented

Code: Select all

-	Translatable strings for title/heading and description. -> will need someone else for that, I am not aware how translations work
-	Use (-1,-1) for cancel, just no error message logged.
-	If filter matches no hex, issue error and select (-1, -1) in a (probably vain) attempt to proceed -> there is validation that there are enough locations available
-	"Select All", "Select None" and "Toggle Selection" options might be good to have.
-	While there should be fixed text about how, you do want to allow the UMC author someplace to explain the why. -> not much space for that, but there is overlay_message key

!	If filter only matches one hex, issue warning and select it automatically, -> with different min and max count, might not be good
!	Options, default 1, for both min and max number of hex the player may select. Results are a table {x,y} or {{x,y},{x,y},...} -> min number added; always double table or {} then

+	Location filter missing: select from entire map
+	Location filter, if present, must match at least one hex.
+	Consider excluding fog/shroud from selection area by default and have options to enable it.
+	I envision an overlay masking non-selection area (ie., dark-gray/black where you can't select, normal map where you can)
+	And I envision another overlay showing what is selected (gold border?) click to toggle.
+	Missing location filter means choose from entire map
+	I am very much opposed to having a confirmation pop-up. If the author wants a conformation dialog, that should be an opt-in behavior at most... better yet let them write it themselves since we shouldn’t encourage such annoyances. -> removed previous confirmation, now only asks if explicitly asked with confirm_message
+	I like the idea of being able to define overlays for selectable, unselectable, hover(selectable), and cancel(hover+un selectable). -> Not sure if anything can be done for hover, but I added support for unselectable overlay. Any overlay can be passed as empty string to not be visible
+	For fog: Just because you cannot see it doesn’t mean your catapult can’t reach it, right? The author can add a vision filter if need be. -> default fog is excluded, but has allow_fog and allow_shroud
+	 I think it should automatically end when you've selected a specified maximum number of locations, but I also think there should be a way to end it early with less than the maximum number of locations. -> selecting selected or unselectable ends early, and at max count finishes now
+	There's no need to fire the original callback -> removed that then
Function code

Code: Select all

--<<

-- Parameters
-- [filter_location] location filter - error if matches no location, if missing matches all locations
-- max_count - optional default 1, how many locations player may select, not allowed to be 0, not allowed to be less than min_count
-- min_count - optional default 1, how many locations player needs to select
-- allow_fog - optional default false
-- allow_shroud - optional default false. If you allow both shroud and fog here, you can include custom vision location filter
-- variable - WML variable for result - optional default location
-- confirm_message - optional, if used then there is dialog to finish selection, with min_count=0 cancel, reseting selection
-- overlay_selected, overlay_selectable, overlay_unselectable - custom overlay - optional, use "" to not display
-- overlay_message - optional printed while selecting
function wesnoth.wml_actions.query_location(cfg)
	local helper = wesnoth.require "lua/helper.lua"
	local T = helper.set_wml_tag_metatable {}
	local location_set = wesnoth.require "lua/location_set.lua"
	-- local overlay_choosable = cfg.overlay_selectable or "terrain/alphamask.png~O(0.2)~CS(255,255,0)"
	local overlay_choosable = cfg.overlay_selectable or "misc/hover-hex.png"
	-- local overlay_chosen = cfg.overlay_selected or "terrain/alphamask.png~O(0.2)~CS(0,255,0)"
	local overlay_chosen = cfg.overlay_selected or "misc/blank-hex.png~BLIT(misc/hover-hex-enemy-bottom.png)~BLIT(misc/hover-hex-enemy-top.png)"
	local overlay_unselectable = cfg.overlay_unselectable or "terrain/alphamask.png~O(0.4)~CS(127,127,127)"
	local variable = cfg.variable or "location"
	local allow_fog = cfg.allow_fog or false
	local allow_shroud = cfg.allow_shroud or false
	local max_count = cfg.max_count or 1
	local min_count = cfg.min_count or 1
	local confirm_message = cfg.confirm_message
	local overlay_message = cfg.overlay_message or "Click on location to choose."
	local filter = helper.literal(helper.get_child(cfg, "filter_location")) or {}
	
	filter.include_borders = false -- prevent getting stuck when only way to get required amount of locations would be with borders, which are not selectable
	if allow_fog and not allow_shroud then
		table.insert(filter, T.filter_vision{side=wesnoth.current.side,respect_fog=false})
	elseif not allow_fog and not allow_shroud then
		table.insert(filter, T.filter_vision{side=wesnoth.current.side})
	end
	
	local allowed_locations = wesnoth.get_locations(filter)
	if #allowed_locations < min_count then helper.wml_error("query_location matches less locations than min_count "..#allowed_locations.."<"..min_count) end
	if min_count > max_count then helper.wml_error("query_location min_count is higher than max_count "..min_count..">"..max_count) end
	if max_count == 0 then helper.wml_error("query_location called with max_count=0") end
	if #allowed_locations == 0 and min_count == 0 then return end
	
	local disabled_locations = location_set.create()
	if overlay_unselectable then
		for _, loc in ipairs(wesnoth.get_locations({{[1]="not",[2]=filter}})) do
			x = loc[1]
			y = loc[2]
			disabled_locations:insert(x, y)
			wesnoth.add_tile_overlay(x, y, { image = overlay_unselectable })
		end
	end
	
	wesnoth.wml_actions.disallow_end_turn()
	
	local res = wesnoth.synchronize_choice("location", function()
		local old_callback = wesnoth.game_events.on_mouse_action
		local finished = false
		local adding = true
		
		local candidates = location_set.create()
		local chosen_locations = location_set.create()
		for _, loc in ipairs(allowed_locations) do
			x = loc[1]
			y = loc[2]
			candidates:insert(x, y)
			wesnoth.add_tile_overlay(x, y, { image = overlay_choosable })
		end
		wesnoth.redraw{}
		
		function wesnoth.game_events.on_mouse_action(x,y)			
			if adding and candidates:get(x,y) then
				if chosen_locations:size() < max_count then
					chosen_locations:insert(x,y)
					candidates:remove(x,y)
					wesnoth.remove_tile_overlay(x, y, { image = overlay_choosable })
					wesnoth.add_tile_overlay(x, y, { image = overlay_chosen })
				end
			elseif adding and chosen_locations:get(x,y) then
				if chosen_locations:size() >= min_count then
					finished = true
				end
			elseif not adding and candidates:get(x,y) then
				finished = true
			elseif not adding and chosen_locations:get(x,y) then
				if chosen_locations:size() > min_count then
					candidates:insert(x,y)
					chosen_locations:remove(x,y)
					wesnoth.remove_tile_overlay(x, y, { image = overlay_chosen })
					wesnoth.add_tile_overlay(x, y, { image = overlay_choosable })
				else
					finished=true
				end
			elseif not candidates:get(x,y) and not chosen_locations:get(x,y) then
				if chosen_locations:size() >= min_count then
					finished = true
				end
			end
			
			if chosen_locations:size() == max_count then
				finished = true
			end
		end
		
		local function print_current_count()
			wesnoth.print{text=overlay_message .. " Selected "..chosen_locations:size().."/ ("..min_count..".."..max_count..")",size=24}
		end
		
		local function clear_selection()
			chosen_locations:iter(function(x,y,data) 
				wesnoth.remove_tile_overlay(x, y, { image = overlay_chosen })
			end)
			chosen_locations:clear()
		end
		
		local function reset_selection()
			chosen_locations:iter(function(x,y,data) 
				wesnoth.remove_tile_overlay(x, y, { image = overlay_chosen })
				wesnoth.add_tile_overlay(x, y, { image = overlay_choosable })
				candidates:insert(x,y)
			end)
			chosen_locations:clear()
			finished = false
		end

		while not finished do
			while not finished do
				print_current_count()
				wesnoth.delay(10)
			end
			
			print_current_count()
			
			if confirm_message then
				local options = {"Finish", "Choose again", "Quit"}
				if min_count ~= 0 then
					table.remove(options, 3)
				end
				local result = helper.get_user_choice({ speaker = "narrator", message = confirm_message }, options)
				if result == 1 then
				elseif result == 2 then
					reset_selection()
				elseif result == 3 then
					clear_selection()
				end
			end
		end
		
		wesnoth.game_events.on_mouse_action = old_callback
		
		candidates:iter(function(x,y,data) 
			wesnoth.remove_tile_overlay(x, y, { image = overlay_choosable })
		end)
		chosen_locations:iter(function(x,y,data) 
			wesnoth.remove_tile_overlay(x, y, { image = overlay_chosen })
		end)
		
		disabled_locations:iter(function(x,y,data) 
			wesnoth.remove_tile_overlay(x, y, { image = overlay_unselectable })
		end)
		disabled_locations:clear()
		
		local wml_object = {}
		for i,v in ipairs(chosen_locations:to_pairs()) do
			wml_object[i] = {"value",{x=v[1],y=v[2]}}
		end
		wesnoth.redraw{}
		
		return wml_object
	end)

	local normal_object = {}
	for i=1,#res do 
		normal_object[i] = res[i][2]
	end
	wesnoth.wml_actions.allow_end_turn()
	helper.set_variable_array(variable, normal_object)
	return normal_object
end

function ql()
	return wesnoth.wml_actions.query_location({T.filter_location{terrain="Cud"}})
end
function qi()
	return wesnoth.wml_actions.query_location({T.filter_location{terrain="Cud"}, overlay_selectable=""})
end
function qo()
	return wesnoth.wml_actions.query_location({T.filter_location{T["not"]{terrain="Cud"}}})
end

function qk()
	return wesnoth.wml_actions.query_location({T.filter_location{terrain="Cud"}, confirm_message="qk"})
end

function qj()
	return wesnoth.wml_actions.query_location({T.filter_location{terrain="Cud"}, max_count=3})
end

function qh()
	return wesnoth.wml_actions.query_location({T.filter_location{terrain="Cud"}, max_count=3,allow_fog=true})
end

function qh()
	return wesnoth.wml_actions.query_location({T.filter_location{terrain="Cud"}, max_count=3,allow_fog=true})
end

-->>

User avatar
Eagle_11
Posts: 701
Joined: November 20th, 2013, 12:20 pm

Re: [engine] Design of [query_location]

Post by Eagle_11 » February 8th, 2018, 9:00 pm

There should be an function for preventing acquiring cloaked target probably.
In the ranged attacks mod im using noticed could be abused as stealth detector to locate ambushed units, however thats only an problem when if done via right-click on target hex having unit or not.

User avatar
Ravana
Moderator
Posts: 1688
Joined: January 29th, 2012, 12:49 am
Location: Estonia

Re: [engine] Design of [query_location]

Post by Ravana » February 8th, 2018, 9:08 pm

That is responsibility of caller to include filter_vision.

User avatar
Celtic_Minstrel
Developer
Posts: 1116
Joined: August 3rd, 2012, 11:26 pm
Contact:

Re: [engine] Design of [query_location]

Post by Celtic_Minstrel » February 9th, 2018, 1:05 am

Ravana wrote:

Code: Select all

+	 I think it should automatically end when you've selected a specified maximum number of locations, but I also think there should be a way to end it early with less than the maximum number of locations. -> selecting selected or unselectable ends early, and at max count finishes now
My expectation on selecting an already-selected hex would be that it deselects it... what you've described here doesn't sound too bad, but I think it could be improved.

As for translatable strings, it's possible they might just work without any extra code.
Author of The Black Cross of Aleron campaign and Default++ era.
Maintainer of Steelhive.

User avatar
Ravana
Moderator
Posts: 1688
Joined: January 29th, 2012, 12:49 am
Location: Estonia

Re: [engine] Design of [query_location]

Post by Ravana » February 9th, 2018, 1:14 am

My expectation on selecting an already-selected hex would be that it deselects it... what you've described here doesn't sound too bad, but I think it could be improved.
If no location filter is passed, all locations are selectable, so there are no unselectable places to click to finish early. Current case only disallows selecting 0, but allows finishing after selecting 1.

I guess normal use cases would add at least some filter, so might be worth logging notice that filter is expected.. like [kill] should warn if [filter] encountered.

Post Reply