User Tag List

Results 1 to 9 of 9

Thread: Set/getComboX Issues Remain

  1. #1
    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,759
    Level
    21
    vBActivity - Bars
    Lv. Percent
    68.68%

    Set/getComboX Issues Remain

    @Saffith :

    What was the reason for this change in all of the Set/GetComboX stuff, and do you remember when you implemented it:

    int m = zc_max((ri->d[1]/10000)-1,0);
    long scr = zc_max(m*MAPSCRS+sc,0);


    to

    int m = (ri->d[1]/10000)-1;
    long scr = m*MAPSCRS+sc;


    I am hearing that this change also breaks older quests.

    Is there any specific record as to what versions of ZC have incorporated these changes? What I am hearing, is that some stuff in 2;53 works, and some stuff needs the older code to work. This may be a critical issue that affects 2.50.0-1, and 2.50.2 quests, differently, and it is starting to make me very, very ill.

    I do not know if this can be properly rectified without the exact codebase for 2.50.0, 2.50.1, and 2.50.2 release builds. Without that code, I have no way to know precisely what changed between versions. Even so, the only way to fix this in 2.53 is going to be to insert a hack, because the behaviour of these functions changed across more 2.50.x builds, than simply 2.50.2 to 2.50.3RC1.

  2. #2
    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,430
    Level
    24
    vBActivity - Bars
    Lv. Percent
    69.55%
    Build 1799. It changed because accepting invalid input isn't a good idea in general, and it wasn't even consistent about what happened. Some invalid values would get adjusted to valid ones, some would do nothing, and an invalid screen number might even end up on a different map.

  3. #3
    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,759
    Level
    21
    vBActivity - Bars
    Lv. Percent
    68.68%
    Quote Originally Posted by Saffith View Post
    Build 1799. It changed because accepting invalid input isn't a good idea in general, and it wasn't even consistent about what happened. Some invalid values would get adjusted to valid ones, some would do nothing, and an invalid screen number might even end up on a different map.

    Build 1799, so, that was after 2.50.2 was released (build 1793, IIRC). I think what can happen because of this, is that if the user sets an invalid reference, and then tries to read it back, that in newer builds, it sets and returns different locations, that may have worked in the past.

    I am also considering that something else may be at fault for what I am seeing in the reports, as one quest in particular, Panoply of Calatia, uses Game->Counter[] and Game->LKeys[] as if they were global variables; and apparently these are returning values in 2.53 that are off by one. @Mitsukara reported that values stored in these are returning one lower than the intended values stored.
    @Saffith :

    I agree with the change, and your logic here. The issue at hand, is that something changed that is affecting heavily scripted quests that reply on these functions, and I need to figure out precisely what the code needs to look like for the quests to run without breaking, and without maintaining issues in ZC that can cause ZC to crash.

    Do you happen to recall if this change was part of this:

    * Fixed incorrect screen calculation and possible crash in Game->SetComboSolid(). ( Saffith, 2015-12-19 19:41:38 )


    Because, as far as I can tell, that was one of the things that was reverted, and that seems dreadful. I wish that there was a simpler solution to all of this, but it might just be easier and cleaner to revert all of these to the state that they were in, during 2.50.2, then add new functions that work properly to 2.54+ and to deprecate the use of these.

    I also do not know what specifically you changed for this log entry:

    * Fixed Game->Get/SetCombo* failing on maps greater than 1. ( Saffith, 2016-02-05 14:29:20 )

    Are the old SVNs for the source still up anywhere, so that I can review changes that occurred prior to the code going up on GH?
    @jman2050 might find that useful, as well.

    If not, do you have snapshots of the 2.50.0, 2.50.1, and 2.50.2 release codebases on your systems?

  4. #4
    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,430
    Level
    24
    vBActivity - Bars
    Lv. Percent
    69.55%
    Quote Originally Posted by ZoriaRPG View Post
    Do you happen to recall if this change was part of this:

    * Fixed incorrect screen calculation and possible crash in Game->SetComboSolid(). ( Saffith, 2015-12-19 19:41:38 )
    It was related to that. COMBOSDM worked differently than the others. It didn't zc_max things up to 0, it didn't subtract one from the map number, and it didn't validate the given map and screen numbers at all. The crash was because it didn't check whether the resulting screen was greater than the total number of screens. Fixing that didn't require changing the others, but the inconsistency annoyed me.

    I also do not know what specifically you changed for this log entry:

    * Fixed Game->Get/SetCombo* failing on maps greater than 1. ( Saffith, 2016-02-05 14:29:20 )
    That was a mistake. Turned out I fixed a problem that hadn't made it into any released builds. Actually, it was an error in that change in 1799; it originally checked scr<MAPSCRS rather than sc<MAPSCRS.

    Are the old SVNs for the source still up anywhere, so that I can review changes that occurred prior to the code going up on GH?
    It's still up, yes. You'd have to talk to DD about access.

  5. #5
    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,759
    Level
    21
    vBActivity - Bars
    Lv. Percent
    68.68%
    Quote Originally Posted by Saffith View Post
    It was related to that. COMBOSDM worked differently than the others. It didn't zc_max things up to 0, it didn't subtract one from the map number, and it didn't validate the given map and screen numbers at all. The crash was because it didn't check whether the resulting screen was greater than the total number of screens. Fixing that didn't require changing the others, but the inconsistency annoyed me.


    That was a mistake. Turned out I fixed a problem that hadn't made it into any released builds. Actually, it was an error in that change in 1799; it originally checked scr<MAPSCRS rather than sc<MAPSCRS.


    It's still up, yes. You'd have to talk to DD about access.

    I spent ~6 hours working with these today. For whatever reason, quests that rely on the functions absolutely depend on them working precisely as they did in 2.50.2 and earlier. For a comparison, try to play ( this quest ) using Beta 10, and then try it in Beta 10 with these redacted to 2.50.2 code.

    I produced > 35 builds today, just trying to sort this specific issue. It was quite ugly, and I would like to have a better resolution. but I already know the way this is going to roll.

  6. #6
    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,430
    Level
    24
    vBActivity - Bars
    Lv. Percent
    69.55%
    It seems to work fine in 2.50.3 RC1 and 2.53 beta 2, so I'm inclined to think that's not really the problem. But I don't know why it makes any difference if that's the case.

  7. #7
    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,759
    Level
    21
    vBActivity - Bars
    Lv. Percent
    68.68%
    Quote Originally Posted by Saffith View Post
    It seems to work fine in 2.50.3 RC1 and 2.53 beta 2, so I'm inclined to think that's not really the problem. But I don't know why it makes any difference if that's the case.
    I really don't know right now. I rolled this stuff back, and I'm letting it sit on simmer. I had complaints that stuff broke, before I did anything remotely related to those functions, which is why I tinkered with them in the first place. I'm inclined to label them as 2.50 glass, and to implement something better in versions after 2.53, that are saner, cleaner, and more well-wrought. Both ComboData, and ScreenData are part of the master plan at this point, so, it might just be better to leave them alone until we have that stuff done.

    I don't know of any quests that would break by reverting them to ~build 1793, but I know of four that break in different ways, by not reverting them, and deducing why, at this point, has drained me entirely of emotion of any sort. I'm in a sort of mechanical fugue state at present, so unless there is some overwhelming reason to further tinker with these things before 2.54 or 2.55 or something truly new, I will probably leave them be in a state that I know is working, and not erratic.

    I may look at how they were set up again in 2.50.3RC1, but AFAIK, nothing changed from 2.53b2 to 2.53b7 that would have made them break. What I did notice, is a completely erratic behaviour pattern; at least when compiled with MSVC9. I'll test the files in beta 2 though, just to see what happens.

    Apparently, MSVC also wanted to completely wreck DrawInteger, so I had to fix that. The static cast of sdci[9] to float, followed by a cast to ( int ) was --somehow-- causing values to output at value-1 when were passed to sprintf(). (ie.g. A value of 5 was printing as 4.)

    How, why? IDK. Casting from long to float to int is ridiculous anyway; so it no longer does that. The only time it static casts to float now, is when you are trying to display decimals.

    Let's see though... I woke at around 03:00GMT, and it's 19:50GMT. Discounting the time that I spent going to market, and preparing my daily meal, I have been working on this mire for at least twelve hours today. I committed all of the changes, applying them to master and 2.50.x branches; except for the reversion of these functions. That is sitting in its own branch, for a day. It was beastly to reconstruct without accurate SVNs of 2.50.2.

    Luckily, I did find some old files that had some ffscript.cpp stuff in them on my RAID array. Now I just want to have me whiskey and stare at something mindless for a while.

  8. #8
    Administrator DarkDragon's Avatar
    Join Date
    Oct 2001
    Posts
    6,228
    Mentioned
    70 Post(s)
    Tagged
    0 Thread(s)
    vBActivity - Stats
    Points
    11,025
    Level
    31
    vBActivity - Bars
    Lv. Percent
    8.15%
    Can we put in compatibility code that restores the old (buggy) behavior for old quests and sanitizes the value for quests built in 2.50.3 or later?

  9. #9
    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,759
    Level
    21
    vBActivity - Bars
    Lv. Percent
    68.68%
    Quote Originally Posted by DarkDragon View Post
    Can we put in compatibility code that restores the old (buggy) behavior for old quests and sanitizes the value for quests built in 2.50.3 or later?
    That would not solve this issue, as you have already detailed for COMBOSM, where you required using ZScript metadata. Opening the quest in 2.53, and saving it with the revised values did not fix it, FWIW; and I'm suspecting that there is something wrong with the allegro libs. Beta 2 was the last version using the 4.4.x libs prior to fixing the race condition. All builds after that point, use your newer allegro lib set, and the other compatibility issues also seem to start with that change; and there were no other changes made that would or could affect these things.

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