User Tag List

Page 2 of 3 FirstFirst 1 2 3 LastLast
Results 11 to 20 of 21

Thread: NEWCOMBOSDM quest rule

  1. #11
    Administrator DarkDragon's Avatar
    Join Date
    Oct 2001
    Posts
    6,228
    Mentioned
    70 Post(s)
    Tagged
    0 Thread(s)
    vBActivity - Stats
    Points
    11,047
    Level
    31
    vBActivity - Bars
    Lv. Percent
    10.69%
    It is a very simple fix, and migration later is not applicable here, as this is a compatibility rule for the engine, that need be read-only in future builds.
    No, I don't understand what you're saying.

    There is nothing wrong with combo solidity in 2.50.2 and earlier, other than (i) failure to bounds-check the value (easy fix that does *not* require any quest rules), and (ii) a slightly aesthetically-displeasing convention for how the parameter is numbered. (ii) is an extremely minor issue and is *not* worth a quest rule to fix if that is the only reason for a change, especially since the numbering has *already* been fixed in master. That change will roll out whenever we publish the next version of ZC. There is no need to rush it out for 2.53.

    If you feel that changing the numbering is an urgent issue, back-port the fix in master to 2.50.x. Do not add new, different, and incompatible mechanics for fixing the same bug. Do not add a new quest rule.

    Similarly, quest rules are added to fix fully-understood compatibility bugs in the code. Not sprinkled here and there "just in case" a bug may or may not be squashed. Saffith says the bitmap alignment issue is resolved (modulo the color problem he pointed out). If this is not the case, please explain the situation where bitmap alignment is still broken, and/or point to a test quest, and we will fix it properly.

  2. #12
    The Timelord
    QDB Manager
    ZC Developer

    Join Date
    Oct 2006
    Location
    Prydon Academy
    Posts
    1,396
    Mentioned
    112 Post(s)
    Tagged
    1 Thread(s)
    vBActivity - Stats
    Points
    4,781
    Level
    21
    vBActivity - Bars
    Lv. Percent
    73.04%
    Quote Originally Posted by DarkDragon View Post
    No, I don't understand what you're saying.

    There is nothing wrong with combo solidity in 2.50.2 and earlier, other than (i) failure to bounds-check the value (easy fix that does *not* require any quest rules), and (ii) a slightly aesthetically-displeasing convention for how the parameter is numbered. (ii) is an extremely minor issue and is *not* worth a quest rule to fix if that is the only reason for a change, especially since the numbering has *already* been fixed in master. That change will roll out whenever we publish the next version of ZC. There is no need to rush it out for 2.53.

    If you feel that changing the numbering is an urgent issue, back-port the fix in master to 2.50.x. Do not add new, different, and incompatible mechanics for fixing the same bug. Do not add a new quest rule.

    Similarly, quest rules are added to fix fully-understood compatibility bugs in the code. Not sprinkled here and there "just in case" a bug may or may not be squashed. Saffith says the bitmap alignment issue is resolved (modulo the color problem he pointed out). If this is not the case, please explain the situation where bitmap alignment is still broken, and/or point to a test quest, and we will fix it properly.
    I'm talking about these notations:

    * Fixed Game->Get/SetCombo* failing on maps greater than 1. ( Saffith, 2016-02-05 14:29:20 )
    * Fixed incorrect screen calculation and possible crash in Game->SetComboSolid(). ( Saffith, 2015-12-19 19:41:38 )

    I know for a fact that SetComboSolid in 2.50.1 and 2.50.2 could cause crashes. If this fix was entirely different to what we are discussing now, then I apologise.
    @Saffith : Was this unrelated?

  3. #13
    Is this the end?
    ZC Developer
    Saffith's Avatar
    Join Date
    Jan 2001
    Age
    41
    Posts
    3,389
    Mentioned
    178 Post(s)
    Tagged
    6 Thread(s)
    vBActivity - Stats
    Points
    6,451
    Level
    24
    vBActivity - Bars
    Lv. Percent
    73.19%
    There were three distinct issues fixed at once:
    - Arguments were not validated
    - Maps numbers started at 0, not 1
    - Screen calculation was based on 128 screens per map, not 136

    The map numbering is only a minor nuisance, but the incorrect screen calculation isn't something users should be expected to work around.

  4. #14
    The Timelord
    QDB Manager
    ZC Developer

    Join Date
    Oct 2006
    Location
    Prydon Academy
    Posts
    1,396
    Mentioned
    112 Post(s)
    Tagged
    1 Thread(s)
    vBActivity - Stats
    Points
    4,781
    Level
    21
    vBActivity - Bars
    Lv. Percent
    73.04%
    Quote Originally Posted by Saffith View Post
    There were three distinct issues fixed at once:
    - Arguments were not validated
    - Maps numbers started at 0, not 1
    - Screen calculation was based on 128 screens per map, not 136

    The map numbering is only a minor nuisance, but the incorrect screen calculation isn't something users should be expected to work around.
    Am I correct in my belief that this COMBOSDM change was also what fixed the 'possible crash', which is what I originally reported happening with SetComboSolid/SetLayerComboS crashing 2.50.2 ?

    There is a reason that I wanted to enforce this distinct behaviour here. Backporting the script headers to this would violate the policy that the script system itself should not differ to 2.50.x for 2.53. Changing the parser and all to use the new system would be problematic, at best, for this release. That is why I went with a QR bit that is set automatically.

  5. #15
    Administrator DarkDragon's Avatar
    Join Date
    Oct 2001
    Posts
    6,228
    Mentioned
    70 Post(s)
    Tagged
    0 Thread(s)
    vBActivity - Stats
    Points
    11,047
    Level
    31
    vBActivity - Bars
    Lv. Percent
    10.69%
    Quote Originally Posted by Saffith View Post
    There were three distinct issues fixed at once:
    - Arguments were not validated
    - Maps numbers started at 0, not 1
    - Screen calculation was based on 128 screens per map, not 136

    The map numbering is only a minor nuisance, but the incorrect screen calculation isn't something users should be expected to work around.
    Agreed; however I don't see this change as part of https://github.com/ArmageddonGames/Z...08edffdcb999d6

    or as being incorporated in the current master code:

    Code:
    case COMBOSDM:
    {
    int pos = (ri->d[0])/10000;
    int sc = (ri->d[2]/10000);
    int m = (ri->d[1]/10000)-1;
    if (curscript->version < 2)
    m++;
    if(pos>=0 && pos<176 &&
    sc>=0 && sc<MAPSCRS &&
    m>=0 && m<map_count)
    {
    long scr = m*MAPSCRS+sc;
    int layr = FFScript::whichlayer(scr);
    if(scr==(currmap*MAPSCRS+currscr))
    ret=(combobuf[tmpscr->data[pos]].walk&15)*10000;
    else if(layr>-1)
    ret=(combobuf[tmpscr2[layr].data[pos]].walk&15)*10000;
    else ret=(combobuf[TheMaps[scr].data[pos]].walk&15)*10000;
    }
    else
    ret = -10000;
    }
    break;
    Do you know when the calculation was changed, and the current state (and need) for compatibility support for it?

  6. #16
    Administrator DarkDragon's Avatar
    Join Date
    Oct 2001
    Posts
    6,228
    Mentioned
    70 Post(s)
    Tagged
    0 Thread(s)
    vBActivity - Stats
    Points
    11,047
    Level
    31
    vBActivity - Bars
    Lv. Percent
    10.69%
    Quote Originally Posted by ZoriaRPG View Post
    There is a reason that I wanted to enforce this distinct behaviour here. Backporting the script headers to this would violate the policy that the script system itself should not differ to 2.50.x for 2.53. Changing the parser and all to use the new system would be problematic, at best, for this release. That is why I went with a QR bit that is set automatically.
    I'm not sure what "policy" you're referring to. It's true we should avoid risky changes; whether adding ZASM version information to .qst file qualifies is up for debate.

    In any case, I've already explained why changes to 2.53 that make quests saved there load incorrectly in master are unacceptable. I'm a little surprised this wasn't self-evident; but in any case, if you send a pull request that fixes the COMBOSDM numbering in 2.53 and fully addresses the three points in my original post, I'd be happy to merge it in. You may want to wait for Saffith to respond, though, since it sounds like there's a third issue with COMBOSDM behavior in old quests that hasn't been addressed in either 2.50.x or master.

  7. #17
    Is this the end?
    ZC Developer
    Saffith's Avatar
    Join Date
    Jan 2001
    Age
    41
    Posts
    3,389
    Mentioned
    178 Post(s)
    Tagged
    6 Thread(s)
    vBActivity - Stats
    Points
    6,451
    Level
    24
    vBActivity - Bars
    Lv. Percent
    73.19%
    Sorry, I guess I misremembered things. It never used the wrong number of screens per map. Maybe I'm thinking of something else.

    So I guess it's just the bounds checking that needs added now.

  8. #18
    Is this the end?
    ZC Developer
    Saffith's Avatar
    Join Date
    Jan 2001
    Age
    41
    Posts
    3,389
    Mentioned
    178 Post(s)
    Tagged
    6 Thread(s)
    vBActivity - Stats
    Points
    6,451
    Level
    24
    vBActivity - Bars
    Lv. Percent
    73.19%
    I'm sure no one cares anymore, but I found what I was thinking of: Game->Get/SetScreenState(). The compiler multiplies the map number by 136, but screen state is stored in game->maps, which uses 128 screens per map. That was accounted for way back in build 1326.

  9. #19
    The Timelord
    QDB Manager
    ZC Developer

    Join Date
    Oct 2006
    Location
    Prydon Academy
    Posts
    1,396
    Mentioned
    112 Post(s)
    Tagged
    1 Thread(s)
    vBActivity - Stats
    Points
    4,781
    Level
    21
    vBActivity - Bars
    Lv. Percent
    73.04%
    Quote Originally Posted by Saffith View Post
    I'm sure no one cares anymore, but I found what I was thinking of: Game->Get/SetScreenState(). The compiler multiplies the map number by 136, but screen state is stored in game->maps, which uses 128 screens per map. That was accounted for way back in build 1326.
    So, this is a lingering bug? If so, I may as well correct it, as long as doing that does not affect old quests. I might roll it forward to 2.54, if it might need a rule.

  10. #20
    Is this the end?
    ZC Developer
    Saffith's Avatar
    Join Date
    Jan 2001
    Age
    41
    Posts
    3,389
    Mentioned
    178 Post(s)
    Tagged
    6 Thread(s)
    vBActivity - Stats
    Points
    6,451
    Level
    24
    vBActivity - Bars
    Lv. Percent
    73.19%
    It's working correctly. No need to do anything.

Thread Information

Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  
About us
Armageddon Games is a game development group founded in 1997. We are extremely passionate about our work and our inspirations are mostly drawn from games of the 8-bit and 16-bit era.
Social