World Conquest II

[tekelili]

hello,
the RNG is broken : the position of the 3 sides is no longer random in map1. if AI is in north-west, side 3 is always in south-east, if AI is in south-west, side 3 is always in north-east, and etc.

There is no documentation about how [generator] randomize starting positions, however I suspect behavior you detected has been normal behavior since classic WC was created. I always suspected some kind of pattern, but probably key "terrain_liked=" in default era factions was breaking sometimes pattern, and certainly is difficult detect a pattern beyond map1.

I think this behavior is more a glitch than a bug, as its effect is quite irrelevant since map2, and in map1 (due to turn order) is somewhat convenient that last player in move reach later to battle to be able to replace allied positions. However, now you pointed it, I will think about fix it in future versions

Edit: I think "randomize sides" option in create game screen works correctly for 3p (it is bugged in 2p). I guess you can use it to fix this issue until I decide harcode a fix

[tekelili]

I expect upload version 0.6.5 next week.

I am posting in advance to warning users that besides my best proposes this version wont be compatible with 0.6.4.4
I will attach 0.6.4.4 to first post after upload. Really sorry for inconvenences to users with several saved games

Among lot of minor fixes and tweaks, what players mostly will notice as changes:
- New alternative map4
- Harcoded randomization of players starting positions to fix [generator] pattern
- All traings changed. All of them very minor tweaks but Ranger Tactics. I have detected a severe lack in its design, +13% damage is quite useless for units like Elvish Fighters, Shamans, Drakes and Orcish Assasins (until they level or get dextrous). It was also not scaling properly. In its redesign I have also added a new custom special: disengage (unit gains 1mp after attack).
I have changed also order of trainings and beneficts. It has some "code" reasons but I also expect improve readability. For players used to old order will be confusing in first instance, but I expect that "read" trait trained will become more intuitive in long run with this new order.

[gfgtdf]

I started playing this campaign and i noticed that the wml menu commands use [allow_undo], this causes OOS (if the action is undone) it is reccomended not to use this.

[tekelili]

@gfgtdf: Thanks a lot for advise, and really in time to allow me fix issue before upload next version

Are those OOS a bug discovered recently or a very well known side effect? I am asking because when I was asked to add [allow_undo] by a user I barely remember checking wiki without find any danger.

Edit: do you think instead remove it I should do this?

Code: Select all

                [if]
                    [variable]
                        name=players
                        equals=1
                    [/variable]
                [then]
                        [allow_undo]
                        [/allow_undo]
                [/then]
                [/if]

Just asking to be really sure

[gfgtdf]

I didn't find an entry in the wiki about this bug so i don't think it was well known. I don't know when i actually noticed it (maybe a half year ago?), I fixed this OOS in 1.13.x 3 months ago, 1.12.5 will not get the same fix. In 1.12.5 the game will simply ignore [allow_undo] inside wml menu commands.

Wesnoth 1.13.2 also supports unsynced wml menu items these are made exactly for things like that and won't change the undo stack

@Edit: hmm yes this will only casue OOS in 1 player games which is not as bad as in mp game but will still casue OOS for replays and observers.

EDIT:

More issues:
- I think it's unclear what the '+moves defence' in the movement training means.
- The code that places the items on the map coudl be improved (the 'wct_bonus_generate_points' takes ~2 second on my pc) (y i know that there are mostllikeley other places too but this is the first i found). This is mostlikeley caused by the default implementation of the radius= filter which is quite slow. I think this can be improved by: Insted of recalculating the filter for each item, you could reuse the point.location variable for each item and then just subtract the items which are now invalid due to the new item.

[gfgtdf]

another issue:
I sometimes get 'Invalid WML found: [if] didn't find any [then], [elseif], or [else] children.' errors. 1.13.x (and also 1.12.x if another addosn backports the 1.13 elseif syntax) does not support those [is]s please fix it.

I alos often get there erros in teh stderr:

- error engine: failed to auto-store $unit at (-999,-999)
- warning engine: attempt to add an [event] with empty id=, ignoring
- warning engine: variable_info: invalid WML array index, player[6].faction.pair.length

[tekelili]

another issue:
I sometimes get 'Invalid WML found: [if] didn't find any [then], [elseif], or [else] children.' errors. 1.13.x (and also 1.12.x if another addosn backports the 1.13 elseif syntax) does not support those [is]s please fix it.

I alos often get there erros in teh stderr:

- error engine: failed to auto-store $unit at (-999,-999)
- warning engine: attempt to add an [event] with empty id=, ignoring
- warning engine: variable_info: invalid WML array index, player[6].faction.pair.length

Thanks a lot for report stderr errors, I guess I should check it more often
I think I can fix some of them (I think [if] without children tags comes from using [insert_tag] name=if from empty variable); but the auto-store unit error and event without id, wich I have no clue where could come from

Edit: I think [event] without id also comes from [insert_tag] an empty variable, but it is more difficult to solve because is used inside [unit]. This is why I did months ago a feature request about [insert_tag] behavior with empty variables (but it was rejected)

If only drawback of those empty [event] tags are stderr errors, then I think I pass to fix it, because I think is [insert_tag] implementation fault and not mine. I can not check variable.length inside [unit], and duplicating [unit] tags inside a conditional looks quite dirty for my taste

Edit2: I will try check placing bonus points implementation, but I fear it will be quite irrelevant: If I am beign inefficient at that process then I am also beign inefficient in map postgeneration wich is more engine demanding and user time consuming relevant
I am beign more focused in things working fine than working fast and I certainly should improve this issue, however is not easy task when time-consuming of terrain filters is not documented and I mostly have to guess it

About "+moves defense" (unit gains +1 defense for each max_moves), I am thinking in add an entry in WoCopedia explaining it. Do you think it is enougth or may be a different name could explain it better?

[tekelili]

I alos often get there erros in teh stderr:

- warning engine: variable_info: invalid WML array index, player[6].faction.pair.length

I think this error is produced by this code

Code: Select all

#define WORLD_CONQUEST_II_ERA_MODIFY_RECRUITING
[event]
    name=recruit
    first_time_only=no
    {FOREACH player[$side_number].faction.pair pair_i}
        [if]
            [variable]
                name=player[$side_number].faction.pair[$pair_i].unit.type
                equals=$unit.type
            [/variable]
        [then]
            {WC_II_SWAP_PAIR player[$side_number].faction.pair[$pair_i]}
            [disallow_recruit]
                side=$side_number
                type=$player[$side_number].faction.pair[$pair_i].replace.type
            [/disallow_recruit]
            [allow_recruit]
                side=$side_number
                type=$player[$side_number].faction.pair[$pair_i].unit.type
            [/allow_recruit]
            {VARIABLE pair_i $player[$side_number].faction.pair.length} ##exit loop
        [/then]
        [/if]
    {NEXT pair_i}
[/event]
#enddef

It happens that array players only has 3 index and event should do nothing when fired by side=6. However I am amused doing a variable conditional over an index that doesnt exist creates an error report... hell, I am doing such operation in lot places. Does stderr really need considere it as an error and report it?

Edit: I have fixed most of problems with empty [if] and [event] inserted, and I think I will fix [event] inside [unit] by checking length, storing unit, using [set_variables] and unstoring... but seriusly, when such amount of ridiculous operations must be done to avoid [insert_tag] crashing things, wouldnt developers reconsidere fix its behavior?

When I did my feature request I think it was not evaluated properly: Imo WML is a quite rusty language for lot of proposes (like not having recursive function implemented). However [insert_tag] makes it extremely powerfull in lot of senses. Creating code dinamically and "iterating" over code is an awesome tool for coders. But for this propose current behavior with nonexisting variables is totally undesired. Who the hell wants a loop that always do an iteration even if variable doesnt exist? Imho [insert_tag] is conceptually a loop and currently is unable to do zero iterations, wich is wrong from any language creation perspective.

I guess due to language level involved, can be a very difficult task fix [insert_tag], but as long it is not impossible I think it should be evaluated as "worth try" because I think [insert_tag] is a masterpiece in WML power

[gfgtdf]

The most important part are the empty [if]s since they do not only give a stderr warning the also throw a lua error on 1.13(which basically aborts the whole event that it appears in).

About the insert_tag: I agree that the behaviour of [insert_tag] with empty vartiable is not what one would at first expect, and that it is in most cases not the most useful behaviour.
But also agree with the reasons said in the bugreport too keep the curent behviour:
1) there is a very easy way to create cutom tag (at least as part of action_wml) that implements this behviour, unfortunatley this wouldn't work in [unit] or similar.
2) The current behaviour is easier to implement.
3) Changing the behviour might break some addons.
For me 1) was the main reason the report it was rejected since the example in the report could be easily fixed by that. I personally never used [insert_tag] in my addons iirc so i don't really know any other cases where the current behviour is a problem then the [event] in [unit] you just mentioned.

2) Is related to how c++ iterstors work: unlike lua iterators which are just a function get_next_element(), c++ iterators have 2 operators move_to_next_element() and get_current_element().

[tekelili]

3) Changing the behviour might break some addons.

I think this concept is not only wrong, but also is totally opossite. Imho only a coder not used to [insert_tag] could be afraid of breaking addons by atending my request. Hypotethical behavior that could be broken is one that have absolute no sense anyone try to achieve with [insert_tag]. However we have right now inverse case: not fixing [insert_tag] has broken my addon from 1.12 to 1.13 by WML not longer ignoring empty [if]

Edit: Saying it with other words: I understand WML not longer supporting empty [if] as enforcing some kind of code philosophy... how does fit in that philosophy having [insert_tag] creating empty tags when variable doesnt exists?

If writting an empty [if] is something so evil that coder should be punished by missfunction... why [insert_tag] is prone to do such evil thing?

[gfgtdf]

I dont know, im neigher the one who changed [if] nor the one who implemented [insert_tag] but mostlikeley those 2 things were done by different people who had different opinions.

Still i think, [if] is not enough reason to change [insert_tag] becasue of 1) from above.

[tekelili]

im neigher the one who changed [if].

i know it, but I think developer responsable of it is currently active, so I would enjoy he explaining me why his feelings about "proper coding" become harmed when I write an empty [if] but he looks comfortable with [insert_tag] beign able to do exactly same

[tekelili]

Uploaded version 0.6.5 (not compaible with previous versions)

@gfgtdf: I fixed empty [if] created by [inset_tag], let me know if add on still has problems in 1.13

[gfgtdf]

@tekelili
i currently have a saved game with the prevois version so i mostlikley won't update until I finished that.

Another thing i notices (in the old version): The savegame names are quite long this causes wesnoth 1.12.4 to remove the mot important part of the savegamne leave leavong only something like "Word conquest 3p Scena..Turn 3" which completley rmeoves the scenarioname or number.
Maybe it is possible to abbreviate "Word conquest 3p Scenario" to just "WC2 3p" or something?

[tekelili]

I agree that long names for save files are a problem (or at least quite annoying). However to be displayed in objectives I prefer be nice and clear rather than short... this remember me I was thinking about do a feature request to use id= as value for one and name= for the otherone, that way I could use a short name and a long one for each propose. Do you think worth a feature request?

Edit: I dont know wich value is used to name created games in multiplayer lobby, but I would prefer name= was used as value.