patchBattle for Wesnoth - Patches: patch #3846, Fixed bug #20734: Default 100 gold...

Show feedback again

patch #3846: Fixed bug #20734: Default 100 gold is applied when gold isn't specified in scenario.cfg

Submitted by:  Mateusz Kołaczek <pl_kolek>
Submitted on:  Fri Apr 12 12:08:30 2013  
Priority: 5 - NormalStatus: In Progress
Privacy: PublicAssigned to: Iurii Chernyi <crab>
Open/Closed: Open

Add a New Comment (Rich MarkupRich Markup):

You are not logged in

Please log in, so followups can be emailed to you.


(Jump to the original submission Jump to the original submission)

Mon May 13 15:38:14 2013, comment #7:

It is the leader's id, not the side one's. Sides don't normally have an id, unless the wml author explicitely specifies a save_id. We could make specifying a non-empty save_id for persistent sides mandatory. If it's not given, stderr message and exit. wmllint could also check it for existance. (For my own campaign I decided that it's mandatory long ago.) This could potentially avoid bugs such as

I recall someone adding [side][leader] but it's not documented in the wiki.

Anonymissimus <anonymissimus>
Project Member
Fri May 10 17:53:56 2013, comment #6:

No, I didn't notice that. From what Crab_ said, I understood that patch such as the first of mine (and probably ayne's patch?) is duplication of the carryover code to other places, and it should be handled just there to avoid maintenance problems.

Judging by the code, I think that we already rely on one of those tags being non-empty. For example:
-transfer_all_to (gamestatus.cpp) checks save_id or id
-teaminfo::read does the same

However, if there are so many ways to just set the side id, then what would you suggest instead?

Mateusz Kołaczek <pl_kolek>
Fri May 10 16:01:03 2013, comment #5:

Did you notice that meanwhile ayne committed a different fix, probably without knowing about this ? (commit 0d2eeeeeefaa8fa87d2861d0ed1011574fda5495)

I think one can't rely on at least one of [side]save_id= or [side]id= not being empty. The leader could also be given by [side][leader]. Perhaps even [side][unit] would work. Or not giving the leader a manual id at all (which is bad of course, since you then cannot reference it).

Anonymissimus <anonymissimus>
Project Member
Thu May 9 17:39:46 2013, comment #4:

Correct version in attachment now.

(file #17941)

Mateusz Kołaczek <pl_kolek>
Tue May 7 19:13:46 2013, comment #3:

The carryover code was not called, because the sides were not initialised in the beginning of the play_campaign function. I created a loop that adds the teams not present.

Patch attached

(file #17918)

Mateusz Kołaczek <pl_kolek>
Fri May 3 05:53:41 2013, comment #2:

Thanks for the patch, but I'd prefer this fixed in a different way. Carryover code should be responsible for setting the gold to default value. That code is located in gamestatus.cpp, and it has the 'set gold to default value' logic. The problem is that this carryover-to-next-scenario code is NOT called before first scenario, but, I'd say that it should be called.

Iurii Chernyi <crab>
Project MemberIn charge of this item.
Fri Apr 12 14:24:16 2013, comment #1:

"I also added gold=100 for SoF:1 for clarity."
I rather wouldn't want that (from the UMC author point of view). Using conventions in mainline tends to outsource workload onto UMC authors, since their addons have the more unusual wml then. Only the fact that this wasn't there made it possible to discover and reproduce the bug with a mainline scenario.

Anonymissimus <anonymissimus>
Project Member
Fri Apr 12 12:08:30 2013, original submission:

Both gold and start_gold are set to default 100 if nothing is specified in config file. Previously only start_gold was being set, which is used only in displaying starting gold in status table.

I also added gold=100 for SoF:1 for clarity.

Mateusz Kołaczek <pl_kolek>


(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):


Depends on the following items: None found

Items that depend on this one: None found


Carbon-Copy List
  • -unavailable- added by crab (Posted a comment)
  • -unavailable- added by anonymissimus (Posted a comment)
  • -unavailable- added by pl_kolek (Submitted the item)

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.


    Error: not logged in



    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu May 9 17:39:46 2013pl_kolekAttached File-=>Added 0001-Fixed-default-100-gold-not-applied-on-the-campaign-s.patch, #17941
    Tue May 7 19:13:46 2013pl_kolekAttached File-=>Added 0001-Fixed-default-100-gold-not-applied-on-the-campaign-s.patch, #17918
    Fri May 3 05:53:41 2013crabAssigned toNone=>crab
    Fri May 3 05:53:40 2013crabStatusNone=>In Progress
    Fri Apr 12 12:08:30 2013pl_kolekAttached File-=>Added 0001-Fixed-bug-20734-gold-is-set-to-default-if-it-s-not-p.patch, #17740
    Show feedback again

    Back to the top

    Powered by Savane 3.1-cleanup