SCI: SQ4 Korean text not rendered after refactor

[sluicebox]

SQ4CD-Korean text no longer appears on the control panel (score, etc) or in the dialog buttons for Quit or Restart, probably others.

This change happened in ​https://github.com/scummvm/scummvm/pull/6003

Before and After pictures of the Quit dialog attached

[athrxx]

Lind of makes sense that something like this would happen in a situation where PC-98 does not have Japanese text. The PC-98 interpreters do have some code changes for the text control to make sure that the text is not overdrawn. But it is limited to situations where PC-98 needs it.

Hopefully, I can transfer these changes to the button control. I mean there could be two possibilities why SCI1 PC-98 does not have Japanese button controls: The devs found it unnecessary/were too lazy or it was too difficult to do it. Let's hope for the first one... :-)

[athrxx]

Okay, I have made a small fix. My impression is that the text control modifications can indeed be easily transferred to other controls. So I made the changes for the button control.

Are there any test cases for icon, list or text edit controls so that we can see if they also need some updating?

Would be good if someone would actually test this more intensely. When I worked on COMI Korean, I did get feedbck from the ScummKOR guys, but I guess the SCI patches are less common (I didn't even manage to find any other than SQ4). I did test the intro skip buttons and quit buttons for SQ4 Korean and SQ4 and KQ5 PC-98, but there might be trickier situation than that...

[sluicebox]

The buttons on Quit/Restart dialogs work again. I also tried the original save/restore dialogs, and those buttons are fixed too. (The input textbox on the save dialog doesn't seem to be a part of the Korean patch either way; it's always behaved normally.)

kDisplay calls don't appear to be drawing anything. This is easiest to test on the control panel screen (screenshots attached), but it also occurs wherever text is drawn directly to the game screen (usually with colors in the original) instead of over the gray message box. Use the debugger to warp to monolith burger (room 386) and every time the manager talks, there should be text on the screen. Clicking talk on him gets a long message.

I'm trying to think where else there's interesting text in this game to test... the hintbook? But I think it's all kDisplay.

[athrxx]

I think kDisplay would affect the PC-98 version, too (unless it has English text there). So maybe I can fix it from the original interpreter disasm.

[athrxx]

Interesting, for PC-98 this is not an issue. It uses a low-res font here. Which also means that the original interpreter does not have a fix for kDisplay.
Fortunately, it can be fixed in the same way as the controls. I am going to make a commit later...

[athrxx]

I have made a fix for kRedraw. The monolith burger scene and the save/load dialog texts now look okay again.

The fix is limited to Korean, since none of the PC-98 interpreters have anything like that and apparently don't need it (this is probably the reason I missed that in the first place, since I did expressly look at all _text16->Box() calls to check whether they had changes to the unusual show argument).

[sluicebox]

Monolith burger and control panel now work (yay!); the Hz. So Good store is still not displaying text. Warp there with room 391, click Do on the monitor at the bottom, then click Instructions.

I believe this is also a bunch of kDisplay calls: ​https://github.com/sluicebox/sci-scripts/blob/main/sq4-cd-dos-1.0/src/rm391.sc

I just saw a Korean bug that is *not* a result of the refactor. Clicking a button (highlighting it) and then mousing away before releasing the click leaves the button empty; the button text is not redrawn. Again, this was introduced with the original Korean commits. English still works. We could make a separate ticket for that.

So far, I'm just casually testing this, it looks like you've got this under control but let me know if you need any help on the script side.

[athrxx]

Replying to sluicebox:

Monolith burger and control panel now work (yay!); the Hz. So Good store is still not displaying text. Warp there with room 391, click Do on the monitor at the bottom, then click Instructions.

I believe this is also a bunch of kDisplay calls: ​https://github.com/sluicebox/sci-scripts/blob/main/sq4-cd-dos-1.0/src/rm391.sc

Yes, that one is a bit tricky. It is a _picNotValid = 1 case, so the drawing is delayed, followed by a scrolling transition. This is not something that the PC-98 versions would do with the hires font. I'll try to come up with a fix that is not too dirty...

[athrxx]

It would require an unusual amount of ugly hacking to support scrolling hires texts. I have worked on it to a point where it really made no sense to continue. It doesn't seem like a good idea to spam the code with KOR hacks, when it takes only 3 or 4 lines of code change to just restore the old state.

The goal of totally getting rid of _upscaledHires cannot be achieved anyway. I have verified that SCI0 Mac really scales coordinates inside the engine drawing code and performs vector operations (like Bresenham line drawing) on these scaled coordinates. So it isn't possible to simply upscale this at a later point.

Is it okay if I partially revert 3ce0915e584f7515513b73a6703a9ba246c1a9d4? And also the two (then obsolete) commits I made here as a fix.

[sluicebox]

Whatever you think is best. I'm not really following the code changes that are going along with this. (That's often the case with the things you do, because they look hard!)

Do you want to sit on it for a while in case you think of something else? None of this is urgent.

My main thing on these extras that weren't in the original, like RTL text or whatever, is that they should be factored out as much as possible from the already complicated genuine engine code by however one can. I believe that's your thing too, so my attitude is "whatever athrxx wants" =)

[athrxx]

Replying to sluicebox:

Whatever you think is best. I'm not really following the code changes that are going along with this. (That's often the case with the things you do, because they look hard!)

Do you want to sit on it for a while in case you think of something else? None of this is urgent.

Yeah, that's probably the best idea. Maybe I can come up with something else during next week or so...

My main thing on these extras that weren't in the original, like RTL text or whatever, is that they should be factored out as much as possible from the already complicated genuine engine code by however one can. I believe that's your thing too, so my attitude is "whatever athrxx wants" =)

Yes, I would really like to factor out the upscale logic, although I have come to understand that for SCI0 Mac there is no chance, since the original interpreter really does have the modified vector drawing code in the engine.

(Unrelated but while you're in the graphics code, did you know that _needsUnditheringPalette isn't used? It generates a warning, but maybe you're going to use it later?)

No, I didn't see any warning. But I think you're right. I am using the argument only in the constructor. Inside remapTextColor() I am just checking the _textModePalette instead. I'll remove the unneeded var...

[sluicebox]

Oh cool, I'm glad SCI0 Mac is on your radar. Last I checked it was corrupting memory all over the place, so I demoted it to unstable. I came away thinking that the comments explaining the priority resolution were wrong (hence all the OOB writes) but it was a while ago. The Dream would be to someday have macgui working in those.

I think we're doing similar things right now. Over in AGI I've been adding support for Apple II games, which change some AGI fundamentals, and it's a lot of work for objectively worse game versions that I can't imagine anyone really caring about. But it's fun and it's the work that's left to do. The RE'ing is sometimes the hard part, but usually it's figuring out Other People's C++ and trying to integrate this "new" (missing!) stuff without devolving the codebase. Each feature is preceded by "cleanup" commits so I can make sense of what's there. Pretty ridiculous effort/reward ratio, but that's unstructured play for you.