My first impression of 1.13

General feedback and discussion of the game.

Moderator: Forum Moderators

Post Reply
shrieker1
Posts: 21
Joined: August 7th, 2017, 5:53 pm

My first impression of 1.13

Post by shrieker1 »

New GUI looks nice. How long did the previous default theme stay? I think it was
time for the new outlook and this seems promising to me. But I have a few
usability concerns with the new design.

-- MP map selection doesn't scale well when I try resize window

I actually have 2x1080p displays extending my desktop but I still prefer using
tiled window placement if possible. But new default theme is hopeless for only
990 window width. I guess pretty much anything sub 1500 width are already
usability issues.

One quick solution would be detection sub X widths and then switching to single
column display with 4 tabs. Then map selection and description wouldn't waste so
much space if user wants to access settings (unlikely scenario). I think click
top bar tab is much easier than trying to scroll the view from scrollbar.

Seems like there is low width variant. Possible improvements:

Automatic switch between low width and full width layouts if user resizes the window

Low width variant seem to have issue with tab bar in top and cancel buttons with 1k width.

Possible improvements for low width layout would be something like:
- tab bar in top center.
- Allow reducing map preview panel width even to narrower width. Quick visual
look seems like 60-70% reduction to minimum width for map preview panel would
still work decently.
- Move create game cancel buttons to be above map list where other important
elements are
- Same for faction settings that would be better placed closer to top where most
of important other settings are.

My basic idea is to have most important stuff at top near to each other while
less often used features and description and map preview would move to bottom
making it very likely that user needs to move mouse a lot less when making
selections.

Same reasoning could be also applied to which tab some game setting belong to.
Some settings are used much more often than others. That makes it good idea to
places most commonly used settings to the front tab directly. Is it possible to
collect statistics from clients or mp server?
That could help choosing better position of specific setting elements than
trying to guess user behavior.

-- Game starting button is placed in far right bottom corner

I think that common use scenario is that user selects map, era and wants to
start the game. For that use scenario all required user interface elements
should as close to each other as possible.

-- strict synchronization would require tooltip to explain it in detail

At least I don't have any good idea what it exactly synchronizes. That
information would be helpful.

-- MP game lobby is using a bit too much vertical space in my opion

Team: <name> takes a lot of vertical space with little value.

-- Locality of faction selection popup

I probably wouldn't have noticed this if MP map selection allowed me to use 1k
window width. But I scaled window to full screen width. The extra width exposed
that opening the faction selection popup to the center of window makes it very
far from the button which opens the popup faction selection is very long if
window is wide. In practical game play they are not so often needed together but
it is more problem for a developer testing scenarios. It is very likely that
scenario testing requires changing settings in both ends of horizontal window
space.

-- Clickable area size for faction selection

IMO it would be logical if user could open faction selection popup from whole
area that includes leader image, leader name, faction name and gender.

-- Distance between player selection drop down menu and

-- Inspect menu slow because,
probably caused by huge number of debug prints about invalid utf-8 and asan enabled build

-- Backward compatible break in multiplayer_side name

multiplayer_side doesn't tell that old description wml syntax doesn't work any
more. It is easy to convert the new one but stale information and a bit
confusing error message with invalid markup syntax can make it a bit harder to
fix than it should be.

Easiest for umc developer would be similar behavior that message gives about
deprecated description wml syntax. That would make it easy to understand and
fix. Specially when multiplayer_side seems to support the image attribute
already in 1.12.

-- set_menu_item description doesn't support & formating character

I haven't yet checked if it support markup formatting instead. markup would
be nice improvement. Silently ignoring old syntax is more a problem here than
possible backward compatible break.

-- Warning scripting/lua: Cannot resolve variable is missing line end

-- I don't think there is any valid way to check if some global lua variable is set already

If lua loading happens to be in multiple preload events from 1.12 era because
embeding lua directly wasn't supported. preload events fire in unspecified order
so initializing some global table from multiple files which put different
attributes to same table required code like

Code: Select all

if not globalvar then
	globalvar = {}
end

globalvar.thisfileattribute = { foo="bar" }
Of course I had already solved that case with only one file registering preload
event and all other using custom event. But I still had a code fragment checking
if bit32 ops are available. That would have implemented a few helpers with bit32
instead of alternatives. But it seems not to be available so I just removed
bit32 variants as dead code.

-- unit_placed event is generated for all units if I use code like

Code: Select all

local function clear_all_unitoverlays()
	ns.debug("clear_all_unitoverlays start")
	ns.WA.remove_unit_overlay {
		image=unitoverlay,
	}
	ns.debug("clear_all_unitoverlays end")
end
I added debug prints around the code because I suspect it was only one able to
cause unit_placed events from my lua unload code. debug prints confirmed that
events come from that code. On that particular case there was no overlays
registered. I would have a simple lua filter function that could filter only
units which have the overlay. But still I suspect I would get bogus unit_placed
events from it.


-- A bonus issue, I had very weird looking wml variable substitution failure with wfl.

I guess I have to look into the formula parser now that I have 1.13 compiled and working.

Error produced:
20170913 08:24:22 error engine: Formula in WML string cannot be evaluated due to Unrecognized token
--> "$side_number"

Code: Select all

							[set_variables]
								mode="append"
								name="temp_build_menu.option"
								[value]
									message="&$NS_system.village[$id].shop_icon|~SCALE($B_SCALE1,$B_SCALE2)=$(concatenate('<b><span color=""#', coloring,'"">$NS_system.village[$id].name</span></b>
', reduce(['','food','wood','stone','gold'],
        concatenate(a, '<span color=""#', if(cost[b] <= current[b], coloring, 'FF0000'),'"">',
            cost[b],symbol[b],'</span> '))
    , ' =
<span color=""#', coloring,'"">$NS_system.village[$id].line1</span>
<span color=""#', coloring,'"">$NS_system.village[$id].line2</span>
<span color=""#', coloring,'"">$NS_system.village[$id].line3</span>
<span color=""#', coloring,'"">$NS_system.village[$id].line4</span>
')
    where coloring = if(
            cost['food'] <= current['food'] and
            cost['wood'] <= current['wood'] and
            cost['stone'] <= current['stone'] and
            cost['gold'] <= current['gold'] and
            adjacent = 'yes',
                'FFFFFF', '666666')
    where cost = ([
            'food' -> $NS_system.village[$id].cost.food,
            'wood' -> $NS_system.village[$id].cost.wood,
            'stone' -> $NS_system.village[$id].cost.stone,
            'gold' -> $NS_system.village[$id].cost.gold
        ]), current = ([
            'food' -> $player[$side_number].food,
            'wood' -> $player[$side_number].wood,
            'stone' -> $player[$side_number].stone,
            'gold' -> $player[$side_number].gold
        ]), symbol = ([
            'food' -> 'f',
            'wood' -> 'w',
            'stone' -> 's',
            'gold' -> 'g'
        ]), adjacent = '$temp_adjacent_match'
    )"
But in next foreach loop nearly identicar success

Code: Select all

							[set_variables]
								mode="append"
								name="temp_build_menu.option"
								[value]
									message="&$NS_system.wall[$id].shop_icon|~SCALE($B_SCALE1,$B_SCALE2)=$(concatenate('<b><span color=""#', coloring,'"">$NS_system.wall[$id].name</span></b>
', reduce(['','food','wood','stone','gold'],
        concatenate(a, '<span color=""#', if(cost[b] <= current[b], coloring, 'FF0000'),'"">',
            cost[b],symbol[b],'</span> '))
    , ' =
<span color=""#', coloring,'"">$NS_system.wall[$id].line1</span>
<span color=""#', coloring,'"">$NS_system.wall[$id].line2</span>
<span color=""#', coloring,'"">$NS_system.wall[$id].line3</span>
<span color=""#', coloring,'"">$NS_system.wall[$id].line4</span>
')
    where coloring = if(
            cost['food'] <= current['food'] and
            cost['wood'] <= current['wood'] and
            cost['stone'] <= current['stone'] and
            cost['gold'] <= current['gold'] and
            adjacent = 'yes',
                'FFFFFF', '666666')
    where cost = ([
            'food' -> $NS_system.wall[$id].cost.food,
            'wood' -> $NS_system.wall[$id].cost.wood,
            'stone' -> $NS_system.wall[$id].cost.stone,
            'gold' -> $NS_system.wall[$id].cost.gold
        ]), current = ([
            'food' -> $player[$side_number].food,
            'wood' -> $player[$side_number].wood,
            'stone' -> $player[$side_number].stone,
            'gold' -> $player[$side_number].gold
        ]), symbol = ([
            'food' -> 'f',
            'wood' -> 'w',
            'stone' -> 's',
            'gold' -> 'g'
        ]), adjacent = '$temp_adjacent_match'
    )"
They both are generated from a macro.

I just tried to run the new settlers on 1.13. I only managed to get game init mostly working. There seems to be too many bugs with older parts of code. I didn't really test anything useful yet.

But it feels like coding is over for today as I managed to make closer to two spelling errors per line when writing a simple function to check which units in a list are adjacent to a village in other list. But at least I got the logic mostly right because it seemed to generate correct values with minimal test.
gfgtdf
Developer
Posts: 1432
Joined: February 10th, 2013, 2:25 pm

Re: My first impression of 1.13

Post by gfgtdf »

shrieker1 wrote:My first impression of 1.13
Which version did you use exactly? There are multiple versions that go under the name of 1.13 (in particular the 1.13.8 release, an eariler release or wesnoth master)
shrieker1 wrote:

New GUI looks nice. How long did the previous default theme stay? I think it was
time for the new outlook and this seems promising to me. But I have a few
usability concerns with the new design.

-- MP map selection doesn't scale well when I try resize window

Note that when you resize the widow while you are in that dialog the dialog often won't relayout correctly, it's recomened to go bak to tiltescreen and open the mp create screen again when doing these tests. A similiar issue applies to all dialogs in wesnoth.
shrieker1 wrote:
-- strict synchronization would require tooltip to explain it in detail
This is an option to debug OOS errors, not sure how useful it is in practise.
shrieker1 wrote:

-- Backward compatible break in multiplayer_side name
-- set_menu_item description doesn't support & formating character

Yes the old DescriptionWML was generally depcreated, i thought we still had a backwards compability path though.
shrieker1 wrote:

-- Warning scripting/lua: Cannot resolve variable is missing line end
Warning scripting/lua: Cannot resolve variable usually means tha you called wesnoth.get/set_variable with a string that is not a valid identifer for a variable like for example a[0][1] or a[[]]
shrieker1 wrote:
-- I don't think there is any valid way to check if some global lua variable is set already
You can still use luas rawget function rawget(_G, "my_global_variable")
shrieker1 wrote:
-- unit_placed event is generated for all units if I use code like

I have no idea what the ns table contains, so i cannot understand your code, but if i undersood the reasoning of unit_playced correctly in genneraly the unit_placed event follows a 'rather fire too often than to rareley' logic and fired whener a unit _might_ be placed, if you have a suggestion on how to improve the unit_placed event you should talk to zookeeper.
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.
shrieker1
Posts: 21
Joined: August 7th, 2017, 5:53 pm

Re: My first impression of 1.13

Post by shrieker1 »

gfgtdf wrote:Which version did you use exactly? There are multiple versions that go under the name of 1.13 (in particular the 1.13.8 release, an eariler release or wesnoth master)
I compiled wesnoth master. To be exact the version number in cache files is v1.13.8+dev (8de5a2e786-Clean)
gfgtdf wrote:This is an option to debug OOS errors, not sure how useful it is in practise.
It sounds like it might be useful for MP scenario developers. But it feels like something that doesn't belong to the first page. But still tool tip to explain how it works and how it can help MP scenario debugging would be nice.
gfgtdf wrote:Yes the old DescriptionWML was generally depcreated, i thought we still had a backwards compability path though.
I saw that [message] and [option] tags produced the warning. But the same deprecation code is missing from other places. If it is easy to make them call same routines it would be nice addition for umc developers porting code from 1.12.
gfgtdf wrote:
shrieker1 wrote:

-- Warning scripting/lua: Cannot resolve variable is missing line end
Warning scripting/lua: Cannot resolve variable usually means tha you called wesnoth.get/set_variable with a string that is not a valid identifer for a variable like for example a[0][1] or a[[]]
I know what issue is causing the warning. But the log message is missing a "\n" from the end. It was making next log messange appear on the same line. The problematic umc code was done in a loop so there was a pretty long line for that particular bug. The warning is really useful but a "\n" in the end would it make it even better.
shrieker1 wrote:
-- I don't think there is any valid way to check if some global lua variable is set already
gfgtdf wrote:You can still use luas rawget function rawget(_G, "my_global_variable")
Thanks. I didn't know rawget yet. I'm pretty new to lua so I probably miss obvious simple solutions because of that. I actually like the new setting which makes it error to read globals without assignment. It exposes a lot bugs and potential bugs in lua code. But it just left me wondering if it prevents checking for existence of global variables.
gfgtdf wrote:
shrieker1 wrote:
-- unit_placed event is generated for all units if I use code like

I have no idea what the ns table contains, so i cannot understand your code, but if i undersood the reasoning of unit_playced correctly in genneraly the unit_placed event follows a 'rather fire too often than to rareley' logic and fired whener a unit _might_ be placed, if you have a suggestion on how to improve the unit_placed event you should talk to zookeeper.
ns.WA is just set_wml_action_metatable. I guess I should have added comment about that detail. But basically it is using [remove_unit_overlay] without any filter. I haven't yet looked into implementation but I suspect that it might call some functions with wrong parameters when removing the unit overlay which causes them to fire the unit_placed event when actually existing units are only modified. I have to study that more and maybe make a bug report about it.
gfgtdf
Developer
Posts: 1432
Joined: February 10th, 2013, 2:25 pm

Re: My first impression of 1.13

Post by gfgtdf »

shrieker1 wrote:
It sounds like it might be useful for MP scenario developers. But it feels like something that doesn't belong to the first page. But still tool tip to explain how it works and how it can help MP scenario debugging would be nice.
Yes i agree that we could place it somehere else, but i have no idea where exactly.
shrieker1 wrote:
I know what issue is causing the warning. But the log message is missing a "\n" from the end. It was making next log messange appear on the same line. The problematic umc code was done in a loop so there was a pretty long line for that particular bug. The warning is really useful but a "\n" in the end would it make it even better.
Ah ok, fixed that.

shrieker1 wrote:
I saw that [message] and [option] tags produced the warning. But the same deprecation code is missing from other places. If it is easy to make them call same routines it would be nice addition for umc developers porting code from 1.12.
Hmm i think it's best if you make a bugreport for that.



shrieker1 wrote: -- A bonus issue, I had very weird looking wml variable substitution failure with wfl.

I guess I have to look into the formula parser now that I have 1.13 compiled and working.

Error produced:
20170913 08:24:22 error engine: Formula in WML string cannot be evaluated due to Unrecognized token
--> "$side_number"
I don't really know the underlying c++ code of that part, but i strongly doubt that changing wall to villages is the reason why it won't work, it'S more likely that that somethign outside is different, maybe one is in a nested even that adds another layer of variable substituion? Or maybe you jut forgot to reload the wml code?
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.
shrieker1
Posts: 21
Joined: August 7th, 2017, 5:53 pm

Re: My first impression of 1.13

Post by shrieker1 »

gfgtdf wrote:Yes i agree that we could place it somehere else, but i have no idea where exactly.
Yesterday I didn't have any good idea either. But today I had something that might work:

- Rename General tab to Game Settings and Game Settings tab to Advanced Settings
- Then move timer settings to the first tab
- Move synchronization check box to third tab

Basic idea there is to provide the first visible tab with all settings that users are most likely to need would be visible immediately. I don't know if user map settings belongs to the first page or not. To me it feels a bit more like advanced setting because it can cause surprising results on some scenarios.

Then bonus UMC addition would be advanced attribute to options. That would allow umc to move some settings to advanced tab too.


Then back to faction selection popup. I think good position for it would be
popup.x = selected_side_row.x
popup.y = std:max(first_side_row.y, selected_side_row.y+selected_side_row.height/2-popup.height/2)
Post Reply