bugBattle for Wesnoth - Bugs: bug #18893, Teleport ability not working...

 
 
Show feedback again

bug #18893: Teleport ability not working properly when applied by object

Submitted by:  David Mikos <coffee>
Submitted on:  Mon 31 Oct 2011 09:10:52 AM UTC  
 
Category: BugSeverity: 3 - Normal
Priority: 1 - LaterItem Group: WML
Status: PostponedPrivacy: Public
Assigned to: Fabian Müller <fendrin>Open/Closed: Open
Release: 1.99+svn(51770)Operating System: Any

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 02 Jan 2012 10:17:35 PM UTC, comment #12:

The fix(es) for this bug introduced other more severe problems so it's reverted now.
To make adding ABILITY_TELEPORT via an object work, use [object]delayed_variable_subbstitution=yes.
Assigning to fendrin (I hope it's clear why).

Anonymissimus <anonymissimus>
Project Member
Mon 02 Jan 2012 10:12:02 PM UTC, SVN revision 52448:

Revert revisions of mine: 51930, 51947

These were the variable substitution delaying in the [object] code. It is
no longer needed due to the workaround [object]delayed_...=yes|no.
Removes repeated wml parsing. Reverts behavior to mostly what was in 1.8
If ABILITY_TELEPORT is added by an [object], or [object][effect][filter]
contains $this_unit, delayed_...=yes is required for the modifications
to work as expected.
However, if the same object wants to get variables substituted and
add ABILITY_TELEPORT to a unit, the code would have worked in 1.8
but doesn't in 1.9.
(bug #18893, bug #19225)

(Browse SVN revision 52448)

Anonymissimus <anonymissimus>
Project Member
Thu 29 Dec 2011 11:27:40 PM UTC, SVN revision 52425:

introduce [object]delayed_variable_substitution=yes|no (def no) (bug #18893, bug #19225)

This isn't pretty since we are repeatedly parsing wml in the common case...

(Browse SVN revision 52425)

Anonymissimus <anonymissimus>
Project Member
Fri 11 Nov 2011 10:29:22 PM UTC, comment #9:

Works for me (on 1.9.10+SVN 51958M).

David Mikos <coffee>
Project Member
Thu 10 Nov 2011 09:18:53 PM UTC, SVN revision 51947:

fix most invalid messages about invalid variable accesses (bug #18893)

(Browse SVN revision 51947)

Anonymissimus <anonymissimus>
Project Member
Wed 09 Nov 2011 06:43:15 PM UTC, comment #7:

No testcase needed. These error messages are invalid. The fix would be to delay variable substitution much more which requires refactoring a lot of called subfunctions. I don't feel like doing it pre-1.10 since this could cause a lot of bugs, so if this is the only problem you'll have to live with these false positives.

Anonymissimus <anonymissimus>
Project Member
Wed 09 Nov 2011 03:16:57 PM UTC, comment #6:

This is pretty bad. Please post your wml testcase.

Anonymissimus <anonymissimus>
Project Member
Wed 09 Nov 2011 04:23:20 AM UTC, comment #5:

It works now. However, the engine spits out the following error on a unit being given an object ("move to" event):

"warning engine: variable_info: retrieving member of non-existent WML container, teleport_unit.side"

David Mikos <coffee>
Project Member
Wed 09 Nov 2011 12:22:25 AM UTC, SVN revision 51930:

delay/add variable substitution in unit::add_modification (fix for bug #18893)

(Browse SVN revision 51930)

Anonymissimus <anonymissimus>
Project Member
Wed 02 Nov 2011 03:07:19 PM UTC, comment #3:

Yes, the previous ABILITY_TELEPORT implementation didn't have any variables to substitute, so the cfg.get_parsed_config() call in game_events.cpp:1863 substitutes variables too early at a time when the wml variable teleport_unit isn't yet set.
We should probably eliminate such call pathes in the engine, that is, passing vconfigs to make variable substitution as late as possible.
(I worked around a similar problem with the [unit] callstack recently.)

Anonymissimus <anonymissimus>
Project Member
Wed 02 Nov 2011 07:10:27 AM UTC, comment #2:

Looks like you are right exasperation. Expanding this macro and making the changes you suggested is a good workaround for the time being. However, changing the abilities.cfg file makes it so that the silver mage, etc. units don't have working teleport. So it is an either or situation with the abilities macro.

David Mikos <coffee>
Project Member
Wed 02 Nov 2011 03:45:03 AM UTC, comment #1:

This may be an issue with premature variable substitution. Try replacing the {ABILITY_TELEPORT} macro with its expansion, but wherever the macro has $teleport_unit use $|teleport_unit instead.

Aaron Keisch-Walter <exasperation>
Mon 31 Oct 2011 09:10:52 AM UTC, original submission:

Teleport works fine when applied to unit as part of unit_type, however fails to work properly when added to a unit via an object. The unit cannot teleport to other flagged villages when on a village and can teleport to unflagged villages when not originally sitting on a village (through unflagged village).

Issue is probably with the new [tunnel] WML tag in abilities.cfg not loading correct details on teleport_unit after the scenario has started.

Code to test would be like:
[object]
silent=yes
duration=forever
[filter]
x,y=$unit.x,$unit.y
[/filter]
[effect]
apply_to="new_ability"
[abilities]
{ABILITY_TELEPORT}
[/abilities]
[/effect]
[/object]

David Mikos <coffee>
Project Member

 

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

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by shadowmaster (Updated the item)
  • -unavailable- added by fendrin (Updated the item)
  • -unavailable- added by anonymissimus (Posted a comment)
  • -unavailable- added by exasperation (Posted a comment)
  • -unavailable- added by coffee (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 13 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 07 May 2014 09:54:10 PM UTCfendrinPriority5 - Normal=>1 - Later
    Wed 07 May 2014 09:48:21 PM UTCfendrinStatusConfirmed=>Postponed
    Fri 11 Jan 2013 10:28:13 PM UTCfendrinStatusNone=>Confirmed
    Sun 29 Jan 2012 09:12:38 PM UTCshadowmasterStatusPostponed=>None
    Mon 02 Jan 2012 10:17:35 PM UTCanonymissimusStatusReady For Test=>Postponed
      Assigned toanonymissimus=>fendrin
    Thu 10 Nov 2011 09:20:24 PM UTCanonymissimusStatusPostponed=>Ready For Test
    Wed 09 Nov 2011 06:44:28 PM UTCanonymissimusStatusIn Progress=>Postponed
    Wed 09 Nov 2011 03:16:57 PM UTCanonymissimusStatusReady For Test=>In Progress
    Wed 09 Nov 2011 12:26:19 AM UTCanonymissimusStatusConfirmed=>Ready For Test
    Sat 05 Nov 2011 12:00:00 AM UTCanonymissimusAssigned tofendrin=>anonymissimus
    Fri 04 Nov 2011 10:05:39 AM UTCfendrinStatusNone=>Confirmed
    Mon 31 Oct 2011 03:05:23 PM UTCanonymissimusAssigned toNone=>fendrin
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup