bugBattle for Wesnoth - Bugs: bug #18877, Transferring units between...

 
 
Show feedback again

bug #18877: Transferring units between campaigns via global variables causes underlying_id conflicts

Submitted by:  SigurdFireDragon <sigurdthedragon>
Submitted on:  Fri 28 Oct 2011 05:00:06 PM UTC  
 
Category: BugSeverity: 2 - Minor
Priority: 3 - LowItem Group: Units
Status: NonePrivacy: Public
Assigned to: Jody Northup <upthorn>Open/Closed: Open
Release: r51685Operating System: Win 7 x64

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 25 Nov 2013 07:45:37 PM UTC, comment #8:

There's an idea to replace the underlying_id with UUIDs. They would be deterministically generated in a scenario, but no other playthrough is likely to ever generate a collision. This wouldn't fix unstores from persistent variables during the same playthrough of the campaign, but that's arguably the author's fault. (and a different bug: you can already do this with regular variables)

We can't really automate the "clear underlying_id" workaround, as persistent variables have no knowledge of what they're holding.

Alexander van Gessel <ai0867>
Project Member
Fri 25 Nov 2011 03:14:28 PM UTC, comment #7:

upthorn, assigning to you as the coder of the persistent variables feature.
Do you think it makes sense or is feasible to implement some check for whether an acquired (or written) global variable is a unit and if yes, clear its underlying_id field ?
Other possible way would be to document it and require the simple workaround of wml coders. Since the engine normally tries to preserve that attribute as much as possible this doesn't seem to be such a bad idea.

Anonymissimus <anonymissimus>
Project Member
Sun 30 Oct 2011 06:07:10 PM UTC, comment #6:

Thanks, clearing underlying_id is simpler & better than how I was doing it.

SigurdFireDragon <sigurdthedragon>
Sun 30 Oct 2011 03:56:16 PM UTC, comment #5:

A suitable workaround should be to clear the underlying_id field of units before transferring them to another campaign. That way their underlying_ids should get adjusted to match the new campaign they're in.

Anonymissimus <anonymissimus>
Project Member
Sun 30 Oct 2011 03:48:18 PM UTC, comment #4:

"Any time someone stores units in persisent variables and unstores them in another game."

Yes, this makes sense.
When a camnpaign is started, the engine gives units the first time they are created ([unit], wesnoth.put_unit, [side] leader definition) underlying_ids. The id counter is increased by one for each new unit created and this is kept over the course of the complete campaign so that underlying_ids are unique (see the next_underlying_unit_id attribute in savegames) When storing/unstoring with persistent vars from/to the same campaign there's no problem but doing that with different campaigns lets of course the underlying_ids collide if the counter of the other campaign is higher....
Apparently nobody thought of this usecase yet when implementing the persistent variables and there should be no difference MP vs SP for the matter.
Please report if you find it under ither circumstances as well.

Anonymissimus <anonymissimus>
Project Member
Sat 29 Oct 2011 06:01:01 PM UTC, comment #3:

Anonymissimus, hopefully the following answers your two posts.

Summary: it does occur in a certain case, may possibly occur in others, and might just be a MP Campaign bug.

The duplicate underlying_id bug does occur without the umc author modifying it. However, this might classify as a MP Campaign Bug (which are currently unsupported)

It can (and does) occur under the following condition:

Any time someone stores units in persisent variables and unstores them in another game. (Might be limited to MP, keep reading.)

This has occured when I was writing my Custom Campaign add-on, and I added code to alter underlying_id specifically to prevent such collisions. (Maybe that means Custom Campaign is a "hacky modification" :D )
I do remeber hearing of at least one other add-on using persistence to allow a player to use their army again for an MP Campaign once they have completed it; this bug might show up there as well.

It might occur under the following conditions:

It's possible that this does not occur in SP, as SP has [side] persistence=yes; which might in it's opperation prevent such things from occuring in how the units are stored between scenarios (I'm not familar with that code to be sure)

In MP, [side] persistence=yes; does not work for any scenario which uses random maps (which is a seperate bug I have yet to draw up a report for, and classifies as an MP Campaign Bug) and add-ons like World Conquest and Random Campaign might have underlying_id collision problems occur because they need to resort to using [store_unit],[unstore_unit] to pass the army between scenarios. (Though there is some linking of scenarios, even though parts are broken, as such their operation is slightly different than those that unstore from a global persistent var.)
I say might occur here because I haven't observed it yet myself, the nature of the bug is such that you have to be paying attention for it, and if your recall list is long, (ie, 15+ units) it would be easy to miss one or two units dissappearing between scenarios) Though the post I linked to below suggests it's occurance in MP Campaigns

SigurdFireDragon <sigurdthedragon>
Fri 28 Oct 2011 09:48:58 PM UTC, comment #2:

In your test case you are modifying underlying_ids. This attribute is the one the engine uses to identify units. WML and lua authors must not modify it (and also should definitely not need to). I also don't know how we should prevent such modification, if the attribute is stored and restored from unit wml variables/tables. So unless you have some evidence that duplicate underlying_ids occurred without underlying_ids being modified, the bug is invalid.

Anonymissimus <anonymissimus>
Project Member
Fri 28 Oct 2011 09:28:35 PM UTC, comment #1:

The question here is how it is possible that you can have the same underlying_id twice. Unless the wml/lua author does some hacky modification this must not be possible.

Anonymissimus <anonymissimus>
Project Member
Fri 28 Oct 2011 05:00:06 PM UTC, original submission:

If two units are the same side and have the same underlying_id, and one is on the map and the other on the recall list...

If the one on the map is put on the recall list, it will overwrite the one already there.

Both wml [unstore_unit] and lua wesnoth.put_recall_unit can cause this.

Noticed this first in developing Custom Campaign, and this post seems to indicate that other MP campaigns have run into this issue, and that it will just occur given enough scenarios.

http://forums.wesnoth.org/viewtopic.php?f=8&t=34170#p496192

I've done a wml workaround, but altering underlying_id strikes me as something a umc author shouldn't have to worry about.

Attached is a test scenario that illustrates the conflict in SP & MP using [unstore_unit]

SigurdFireDragon <sigurdthedragon>

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #14301:  Test_Bed.zip added by sigurdthedragon (1kB - application/x-zip-compressed)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by ai0867 (Posted a comment)
  • -unavailable- added by anonymissimus (Posted a comment)
  • -unavailable- added by sigurdthedragon (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.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 19 Jan 2012 06:20:28 PM UTCanonymissimusSummaryPutting a unit on the recall list can overwrite one already on the list =>Transferring units between campaigns via global variables causes underlying_id conflicts
    Fri 25 Nov 2011 03:14:28 PM UTCanonymissimusAssigned toNone=>upthorn
    Mon 31 Oct 2011 03:08:25 PM UTCanonymissimusSeverity3 - Normal=>2 - Minor
      Priority5 - Normal=>3 - Low
    Fri 28 Oct 2011 05:00:06 PM UTCsigurdthedragonAttached File-=>Added Test_Bed.zip, #14301
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup