NEWCOMBOSDM quest rule

[DarkDragon]

I've removed it from 2.50.x:

1. It was not correctly incorporated, since it was added to 2.50.x without updating master. It is a complete disaster for the two branches to use the same quest rule bit for different purposes! (all quests saved in 2.50.x would work incorrectly in future ZC versions).

2. The same issue is already fixed in master using a different mechanism (ZASM version metadata, rather than quest rule). Master's mechanism needs to be removed and replaced with whatever 2.50.x does, if 2.50.x is changed to do something different.

3. From the threads here and on PureZC it doesn't sound like anyone will be affected by just rolling back the 2.50.3 COMBOSDM behavior.

If you need to put the quest rule back in for some reason, please ensure points #1 and #2 are fully addressed.

[ZoriaRPG]

I've removed it from 2.50.x:

1. It was not correctly incorporated, since it was added to 2.50.x without updating master. It is a complete disaster for the two branches to use the same quest rule bit for different purposes! (all quests saved in 2.50.x would work incorrectly in future ZC versions).

Of course I did not add it to master. You explicitly told me not to make any internal changes for master until you were done with your own. I can't set up rules changes in ZDefs.h or in FFRules until you finish what you are doing, but I was planning to forward this to master, and to 2.54, later, as that is still an interim goal.

2. The same issue is already fixed in master using a different mechanism (ZASM version metadata, rather than quest rule). Master's mechanism needs to be removed and replaced with whatever 2.50.x does, if 2.50.x is changed to do something different.

3. From the threads here and on PureZC it doesn't sound like anyone will be affected by just rolling back the 2.50.3 COMBOSDM behavior.

If you need to put the quest rule back in for some reason, please ensure points #1 and #2 are fully addressed.

The change to COMBOSDM is critical, as the old code causes ZC to crash if it is used on layers higher than 1. It is not merely there to support 2.50.3RC1 quests, but to address a critical bug. The QR allows us a way to permit this fix in releases until 2.60 is ready, which as far as I can see, is stretching farther into the future, every day. Adding an inclusive statement to check for this old bit or a script header version > blah, should suffice.

When you are done with what you are doing, and I can properly address the rules for 2.60, including porting enemy stuff to the enemy classes, item stuff to the item classes, and all of that fun, this can just sit in there under old_rules as the second-to-last-bit. We also need a bit for bitmap positioning. That was another debacle intended as a silent 'fix' for an old 'bug', that instead became its own huge bug/mistake, introduced in 2.50.2 that fragmented the community by making some very popular quests [U]unplayable[U] in 2.50.2.

I developed a header just to determine the version between 2.50., 2.50.1, and 2.50.2, expressly because of that problem (see: ZVersion.zh) that was used to patch several quests.

I truly want to be able to tell everyone that all quests made in any 2.50.x version are safe in 2.53. I don't want users to need exceptions, or excuses, to avoid updating for fear that Project-X is broken, as this is what happened with absolutely every update to 2.50.0.

Otherwise, what the flidd is the point of spending the time to make it?

I did not handle the change in this manner, on a whim.

[DarkDragon]

Of course I did not add it to master. You explicitly told me not to make any internal changes for master until you were done with your own. I can't set up rules changes in ZDefs.h or in FFRules until you finish what you are doing, but I was planning to forward this to master, and to 2.54, later, as that is still an interim goal.

You are right, however this kind of change is not a major change to internal quest data structures and the branches cannot be allowed to lose backward-compatibility with respect to each other ("we'll remember to fix it some time in the future" is a dangerous game). If you ask for help I can make sure that changes are properly synched in both branches.

The change to COMBOSDM is critical, as the old code causes ZC to crash if it is used on layers higher than 1. It is not merely there to support 2.50.3RC1 quests, but to address a critical bug.

As I see it there are two separate issues: (i) ZC crashes when passed an invalid layer; (ii) the indexing is off-by-one from the ideal numbering system.

Fixing (i) poses no compatibility issues and requires no quest rule, and can be safely applied to 2.50.x (and master, why not?) Fixing (ii) is the more tricky issue but, as far as I can tell, far from urgent. If you think there really must be a renumbering in 2.50.x, the options are (a) apply the fix in master (based on ZASM versioning) to 2.50.x; (b) remove the fix in master based on ZASM versioning, and add a quest rule (with consistently-placed bits in both branches!!) to both branches. Changing 2.05.x (only) to do its own thing is not a possibility as it breaks quest compatibility.

We also need a bit for bitmap positioning. That was another debacle intended as a silent 'fix' for an old 'bug', that instead became its own huge bug/mistake, introduced in 2.50.2 that fragmented the community by making some very popular quests [U]unplayable[U] in 2.50.2.

I do not know the details about this issue. My preferences, in descending order, are (i) restore 2.50.2 behavior and drop whatever was incorrectly changed in 2.50.3 completely. If this breaks 2.50.3 quests in a non-moot way (somebody's work will actually be affected by the rollback), add compatibility logic to master so that 2.50.3 quests will play correctly on master. (ii) carefully add logic to both 2.50.x and master that preserves behavior of 2.50.3 quests while fixing the 2.50.2 compatibility bug.

I did not handle the change in this manner, on a whim.

I understand, yet regardless of the intent with which it was made, the status quo is unacceptably broken code, for the reasons I explained above and in my original post. The current state of the repository (quest rule completely rolled back) is acceptable in my mind provided that nobody steps forward with concrete evidence that a real-world quest will be affected. If you are worried, you may send a new patch request that fixes the 2.50.3 solidity command and addresses the three concerns in my original post.

[Saffith]

I do not know the details about this issue. My preferences, in descending order, are (i) restore 2.50.2 behavior and drop whatever was incorrectly changed in 2.50.3 completely. If this breaks 2.50.3 quests in a non-moot way (somebody's work will actually be affected by the rollback), add compatibility logic to master so that 2.50.3 quests will play correctly on master. (ii) carefully add logic to both 2.50.x and master that preserves behavior of 2.50.3 quests while fixing the 2.50.2 compatibility bug.

In 2.50.1 and before, when switching drawing targets between the screen and an offscreen bitmap, the subscreen offset wouldn't always be set properly, so drawing would sometimes incorrectly be shifted by 56 pixels. 2.50.2 fixed that without keeping it for old quests; 2.50.3 does restore it in old quests.
That's currently handled by a bit set at the end of readrules() (in 2.50.x, it's set at qst.cpp:2549 and read at script_drawing.cpp:2159). It's based entirely on the version and build; there's no quest rule or anything else saved with the quest file. Quests made in 2.50.3 will function the same in 2.50.2 in this regard.

[DarkDragon]

The issue then is that the fix wasn't also applied to master? Because that behavior sounds ideal to me.

[Saffith]

No, it's also in master.

[DarkDragon]

I'm afraid I'm confused about what the problem is then... @ZoriaRPG can you elaborate?

In any case, Zoria, the TL;DR of this thread is that any change must

1. preserve backward-compatibility of 2.50.x with respect to quests saved in old versions of ZQ;
2. preserve backward-compatibility of master with respect to 2.50.x (from the perspective of master, 2.50.x *is* an "old version" even though it's not released yet!)

These are "iron laws" in the sense that a change that breaks one is almost always a bad change, regardless of its other merits. I rolled back the NEWCOMBOSDM rule because it ran afoul of (2).

[ZoriaRPG]

I'm afraid I'm confused about what the problem is then... @ZoriaRPG can you elaborate?

In any case, Zoria, the TL;DR of this thread is that any change must

1. preserve backward-compatibility of 2.50.x with respect to quests saved in old versions of ZQ;
2. preserve backward-compatibility of master with respect to 2.50.x (from the perspective of master, 2.50.x *is* an "old version" even though it's not released yet!)

These are "iron laws" in the sense that a change that breaks one is almost always a bad change, regardless of its other merits. I rolled back the NEWCOMBOSDM rule because it ran afoul of (2).

Is it safe to edit zdefs.h, ffscript.cpp, and the jwin panes to add this rule to master? If not, then this is just delaying 2.53 indefinitely, until it is safe to edit those files.

If so, I want to get the bitmap offset fix rule in at the same time, as that also needs to go into 2.53.

It would have been nice to alert me of this issue before adding a reversion push. I will need to re-do the same work, again, to reverse it, although given that I need to add in a rule for bitmaps, that is less of an added workload. Still, if it was safe to edit master, then you could have asked me to fix it there, instead. 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. ( Setting it for 2.60 makes no sense.)

I will probably invert it though.

[ZoriaRPG]

In 2.50.1 and before, when switching drawing targets between the screen and an offscreen bitmap, the subscreen offset wouldn't always be set properly, so drawing would sometimes incorrectly be shifted by 56 pixels. 2.50.2 fixed that without keeping it for old quests; 2.50.3 does restore it in old quests.
That's currently handled by a bit set at the end of readrules() (in 2.50.x, it's set at qst.cpp:2549 and read at script_drawing.cpp:2159). It's based entirely on the version and build; there's no quest rule or anything else saved with the quest file. Quests made in 2.50.3 will function the same in 2.50.2 in this regard.

Unfortunately, this did not make it into the 2.50.x branch, from the reports that I am getting. I think that having both a version check, and a QR is the safest way, with the version check setting the bit on quest init. This applies both to bitmap drawing, and COMBOSDM. That way, the bit is all that matters for future versions; and if we use a script header for some of these fixes, then the bit reading can be an inclusive or condition to evaluate alongside the script header. If the scritpt header is < n, check for the bit. If the bit is off, use the older of the two methods.

I do not know why the fix that was in 2.50.3 is not here, or not working. Is this another case of my files missing a commit, or is it truly missing in the repo?

[Saffith]

I got the line numbers from the repo, so it's definitely there.

Here's a test quest made in 2.50.1: https://www.dropbox.com/s/8ku2k4hku7...pTest.qst?dl=0
It tries to draw three tiles at the same Y position.
In 2.50.1, they're misaligned.
In 2.50.2, they're aligned.
In 2.50.3, they're misaligned.
In 2.53 beta 2, the're misaligned.
So it appears to be working as expected in both 2.50.3 and 2.53.


I'm also seeing that the middle tile is sometimes drawn with a black background and sometimes not, at least in 2.50.2 and up. That should probably be looked into...