[engine] Show [chat] to observers

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

Moderator: Forum Moderators

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

[engine] Show [chat] to observers

Post by Ravana »

Currently function wml_actions.chat(cfg) works by calling wesnoth.message, if the client executing it is found in side-filtered sides. Since observer never controls sides, message is never displayed for observers.

I propose adding new optional key to [chat], to control if message should be visible to observers. I have implemented it as

Code: Select all

function wml_actions.chat(cfg)
	local side_list = wesnoth.get_sides(cfg)
	local message = tostring(cfg.message) or
		helper.wml_error "[chat] missing required message= attribute."

	local speaker = cfg.speaker
	if speaker then
		speaker = tostring(speaker)
	else
		speaker = "WML"
	end

	for index, side in ipairs(side_list) do
		if side.controller == "human" then
			wesnoth.message(speaker, message)
			break
		end
	end
	
	local observable = cfg.observable or true
	if observable then
		local all_sides = wesnoth.get_sides()
		local has_human_side = false
		for index, side in ipairs(all_sides) do
			if side.controller == "human" then
				has_human_side = true
				break
			end
		end
		
		if not has_human_side then
			wesnoth.message(speaker, message)
		end
	end
end
User avatar
Celtic_Minstrel
Developer
Posts: 2166
Joined: August 3rd, 2012, 11:26 pm
Location: Canada
Contact:

Re: [engine] Show [chat] to observers

Post by Celtic_Minstrel »

The idea is good, but I don't understand how this code is expected to work. Won't the message be displayed twice in some cases? Also, your cfg.observable or true will mean that observable is always true.
Author of The Black Cross of Aleron campaign and Default++ era.
Former maintainer of Steelhive.
User avatar
Ravana
Forum Moderator
Posts: 2949
Joined: January 29th, 2012, 12:49 am
Location: Estonia
Contact:

Re: [engine] Show [chat] to observers

Post by Ravana »

Oh yes, I need to figure out how to use cfg variable there still. I initially had it default to false, but with true it is much easier to test that I actually see messages.

Message will not be displayed twice. Original loop shows message only to clients, that have local human side. My addition shows message only to clients that have no local human side.

It gets new list of all sides, to avoid the case where local human side is intentionally side-filtered out from original loop, and checks that none of sides is local.
User avatar
Celtic_Minstrel
Developer
Posts: 2166
Joined: August 3rd, 2012, 11:26 pm
Location: Canada
Contact:

Re: [engine] Show [chat] to observers

Post by Celtic_Minstrel »

Ah, that makes sense. I'm guessing you've also confirmed that it actually works?

If you want it to default to false, then `cfg.observable or false` would work, though so would just plain `cfg.observable`. If you want it to default to true you need something like `cfg.observable ~= false`.
Author of The Black Cross of Aleron campaign and Default++ era.
Former maintainer of Steelhive.
User avatar
Ravana
Forum Moderator
Posts: 2949
Joined: January 29th, 2012, 12:49 am
Location: Estonia
Contact:

Re: [engine] Show [chat] to observers

Post by Ravana »

I tested that I see messages. Clearly not yet that the key works.
User avatar
Celtic_Minstrel
Developer
Posts: 2166
Joined: August 3rd, 2012, 11:26 pm
Location: Canada
Contact:

Re: [engine] Show [chat] to observers

Post by Celtic_Minstrel »

Yeah, that's what I meant – confirming that it lets you see the messages as an observer.
Author of The Black Cross of Aleron campaign and Default++ era.
Former maintainer of Steelhive.
User avatar
Ravana
Forum Moderator
Posts: 2949
Joined: January 29th, 2012, 12:49 am
Location: Estonia
Contact:

Re: [engine] Show [chat] to observers

Post by Ravana »

With 1.13.8 it seems to show message to everyone, despite used side filter. Will be testing with vpn.

https://github.com/wesnoth/wesnoth/issues/1735
User avatar
Ravana
Forum Moderator
Posts: 2949
Joined: January 29th, 2012, 12:49 am
Location: Estonia
Contact:

Re: [engine] Show [chat] to observers

Post by Ravana »

Version with debug statements

Code: Select all

function wml_actions.chat(cfg)
	local side_list = wesnoth.get_sides(cfg)
	local speaker = tostring(cfg.speaker or "WML")
	local message = tostring(cfg.message or
		helper.wml_error "[chat] missing required message= attribute."
	)

	for index, side in ipairs(side_list) do
		if side.controller == "human" and side.is_local then
			wesnoth.message(speaker, message)
			break
		end
	end
	
	local observable
	if cfg.observable ~= nil then
		observable = cfg.observable
	else
		observable = true
	end
	wesnoth.message("observable is "..tostring(observable))
	
	if observable then
		local all_sides = wesnoth.get_sides()
		local has_human_side = false
		for index, side in ipairs(all_sides) do
			wesnoth.message("side "..tostring(index).." cnt "..tostring(side.controller.." local "..tostring(side.is_local)))
			if side.controller == "human" and side.is_local then
				has_human_side = true
				break
			end
		end
		
		if not has_human_side then
			wesnoth.message(speaker.." Rav obs", message)
		end
	end
end
and clean one

Code: Select all

function wml_actions.chat(cfg)
	local side_list = wesnoth.get_sides(cfg)
	local speaker = tostring(cfg.speaker or "WML")
	local message = tostring(cfg.message or
		helper.wml_error "[chat] missing required message= attribute."
	)

	for index, side in ipairs(side_list) do
		if side.controller == "human" and side.is_local then
			wesnoth.message(speaker, message)
			break
		end
	end
	
	local observable
	if cfg.observable ~= nil then
		observable = cfg.observable
	else
		observable = true
	end
	
	if observable then
		local all_sides = wesnoth.get_sides()
		local has_human_side = false
		for index, side in ipairs(all_sides) do
			if side.controller == "human" and side.is_local then
				has_human_side = true
				break
			end
		end
		
		if not has_human_side then
			wesnoth.message(speaker, message)
		end
	end
end
Everything should work correctly now.

This uses side.is_local, which I think should be on https://wiki.wesnoth.org/LuaWML/Sides#wesnoth.sides.
I tested 9 configurations

Code: Select all

[option]
	label=_"chat default side 1"
	[command]
		[chat]
			message=_"default side 1"
			side=1
		[/chat]
	[/command]
[/option]
[option]
	label=_"chat observable=yes side 1"
	[command]
		[chat]
			message=_"chat observable=yes side 1"
			observable=yes
			side=1
		[/chat]
	[/command]
[/option]
[option]
	label=_"chat observable=no side 1"
	[command]
		[chat]
			message=_"chat observable=no side 1"
			observable=no
			side=1
		[/chat]
	[/command]
[/option]
[option]
	label=_"chat default side 1 2"
	[command]
		[chat]
			message=_"default side 1 2"
			side=1,2
		[/chat]
	[/command]
[/option]
[option]
	label=_"chat observable=yes side 1 2"
	[command]
		[chat]
			message=_"chat observable=yes side 1 2"
			observable=yes
			side=1,2
		[/chat]
	[/command]
[/option]
[option]
	label=_"chat observable=no side 1 2"
	[command]
		[chat]
			message=_"chat observable=no side 1 2"
			observable=no
			side=1,2
		[/chat]
	[/command]
[/option]
[option]
	label=_"chat default"
	[command]
		[chat]
			message=_"default"
		[/chat]
	[/command]
[/option]
[option]
	label=_"chat observable=yes"
	[command]
		[chat]
			message=_"chat observable=yes"
			observable=yes
		[/chat]
	[/command]
[/option]
[option]
	label=_"chat observable=no"
	[command]
		[chat]
			message=_"chat observable=no"
			observable=no
		[/chat]
	[/command]
[/option]
gfgtdf
Developer
Posts: 1432
Joined: February 10th, 2013, 2:25 pm

Re: [engine] Show [chat] to observers

Post by gfgtdf »

I'd replace

Code: Select all


   local observable
   if cfg.observable ~= nil then
      observable = cfg.observable
   else
      observable = true
   end
with local observable = cfg.observable~= false. Othwewise the code looks good ot me, please file a pull request on https://github.com/wesnoth/wesnoth/pulls

This uses side.is_local, which I think should be on https://wiki.wesnoth.org/LuaWML/Sides#wesnoth.sides.
Hmm ye woudl be nice it you'd add it tpo the wiki.
Scenario with Robots SP scenario (1.11/1.12), allows you to build your units with components, PYR No preperation turn 1.12 mp-mod that allows you to select your units immideately after the game begins.
User avatar
Ravana
Forum Moderator
Posts: 2949
Joined: January 29th, 2012, 12:49 am
Location: Estonia
Contact:

Re: [engine] Show [chat] to observers

Post by Ravana »

Opened, https://github.com/wesnoth/wesnoth/pull/1739

I wrote what I could guess/find out about is_local. I dont have enough information to update controller entry though.
is_local: boolean (read (Version 1.13.3 and later only)) whether the side is local
gfgtdf
Developer
Posts: 1432
Joined: February 10th, 2013, 2:25 pm

Re: [engine] Show [chat] to observers

Post by gfgtdf »

thx, if noone objects in the next 24 hours i'll merge it.
is_local: boolean (read (Version 1.13.3 and later only)) whether the side is local
i'd put the 'Version 1.13.3 and later only' before the 'read' but otherwiese it's fine.
Scenario with Robots SP scenario (1.11/1.12), allows you to build your units with components, PYR No preperation turn 1.12 mp-mod that allows you to select your units immideately after the game begins.
User avatar
Celtic_Minstrel
Developer
Posts: 2166
Joined: August 3rd, 2012, 11:26 pm
Location: Canada
Contact:

Re: [engine] Show [chat] to observers

Post by Celtic_Minstrel »

Pretty sure is_local is new in 1.13.8 or maybe 1.13.7, not in 1.13.3.

Also note that using it carelessly will definitely cause out-of-sync errors. Maybe that's obvious from the name, but it wouldn't hurt to state it explicitly.
Author of The Black Cross of Aleron campaign and Default++ era.
Former maintainer of Steelhive.
User avatar
Ravana
Forum Moderator
Posts: 2949
Joined: January 29th, 2012, 12:49 am
Location: Estonia
Contact:

Re: [engine] Show [chat] to observers

Post by Ravana »

I found reference to is_local from https://github.com/wesnoth/wesnoth/comm ... 49d84784e4.

Updated
is_local (Version 1.13.3 and later only): boolean (read). Whether the side is local. Careless use will cause OOS errors.
User avatar
Celtic_Minstrel
Developer
Posts: 2166
Joined: August 3rd, 2012, 11:26 pm
Location: Canada
Contact:

Re: [engine] Show [chat] to observers

Post by Celtic_Minstrel »

Yes, but it wasn't exposed to Lua until https://github.com/wesnoth/wesnoth/comm ... b84e0f3ad9
Author of The Black Cross of Aleron campaign and Default++ era.
Former maintainer of Steelhive.
Post Reply