ingwarsw / tinyfugue

TinyFugue - Rebirth
Other
62 stars 34 forks source link

ICU config #39

Closed ingwarsw closed 3 years ago

ingwarsw commented 3 years ago

Thats why its also using pkg-config..

Your version dont set include/link paths and other things..

ingwarsw commented 3 years ago

@warmstarter I just found some time to seriously test it.. (I had to upgrade my whole server to install ICU libraries)

And results are ok when enable-widechar is on but when disable-widechar everything is treated as prompt.. Could you check it?

warmstarter commented 3 years ago

@warmstarter I just found some time to seriously test it.. (I had to upgrade my whole server to install ICU libraries)

And results are ok when enable-widechar is on but when disable-widechar everything is treated as prompt.. Could you check it?

Can you explain a bit more by what you mean by 'everything is treated as prompt' ? Perhaps a better understanding of what other options you might have enabled or disabled, a screenshot of what you are you expecting to see vs what you are expecting to see, the various versions of system libraries that you are using with this and not using with this. Then also what happens when you run "tf -f" vs "tf" so we know if it's happening by default or perhaps when interacting with a script you are running (not saying the script itself would necessarily be the fault, but it might be interacting with something.)

To give a but of the timeline: 2012 - Initial ICU/UTF8/Widechar patch 2014 - Patch brought into your release 2015 - Bug mentioned fixed in one fork 2021 - Bug fix gets merged into your fork and the other forks supporting ICU/UTF8/Widechar, along with a nasty and simple decode_attr bug found in the process.

So the patch itself is pretty well tested for the past 6 years. But there's definitely additional modifications that have been made since then which might be interacting badly with the patch, so trying to better understand what they might be. It's also simply possible that the types of MU*s servers the others had been using and testing on are different and because of that there's a bug that existed for some time in this patch, but was not noticed.

I noticed there's part of your Python patch that seems to rely on WIDECHAR, but does not properly do a check for it being enabled, so that could potentially be an issue. Have you in general been using your fork with WIDECHAR enabled, and are just testing without it enabled, or were you not using it at all until now?

warmstarter commented 3 years ago

42 fixes the issues I mentioned. Not sure if this is the issue that is causing you problems, but it's definitely an issue in your fork that relates to the same class of problems.

ingwarsw commented 3 years ago

@warmstarter Im testing on pure tf (no scripts) Tested on linux and mac..

./src/tf -Llib/tf genesismud.org 3011

I also tested in on second LP mud that im playing and result is exactly the same. (so it may be LP prompt handling issue)

And you will see all output put into prompt (going under visual line)..

Result with disable-widechar image

Result with enable-widechar and on main branch image

warmstarter commented 3 years ago

Ok, I haven't used games with prompts before, and I don't think the person who did the original widechar fix patch did either.

I'll see what I can do about looking into figuring this out.

I think there's two things I'd look into first: There an 'lpflag' and possibly there's something there that needs to be set.

There are also different kinds of *printf() used by TF, and I actually just figured almost the opposite of that (and this is a patch you might want to look into applying yourself.)

In tfpython.c at the top of the file there's an option for a python debugger that you can enable. I didn't like that it was a compile time option, but even the way it worked, it would printf it's debug errors bellow the status line as opposed to above. I understand why you'd want that for a prompt, but for a debugger it wasn't really something that was useful.

Check out this commit, as it might hold some answers, plus you might want to apply it for other reasosn:

https://github.com/warmstarter/tinyfugue/commit/23dbb6ef48ad38f1b0d5f8bc2b2564e2fd71d1d7

The original python debugger used 'oprintf' which goes below the visual line. I created a new flag and a variant of tfwprintf called tfpywprintf. If you don't want a Python debugger that you can toggle on/off in the game, just changing the oprintf in tfpython.c to tfwprintf actually makes it useable.

Point is that I think it's something like that going on, but I'll have to look through that patch a little more and get an idea of how LPs work, but Ill put it on my list. Let me know if any of that makes sense.

I also wonder if any of this is the python patch that's the problem. Presumably you were using the buggy --enable-widechar before with the python. Have you checked if this issue exists in the fork that only has the fixed widechar without python? That would help narrow down where this is going from:

https://github.com/sketch/tinyfugue

warmstarter commented 3 years ago

I tried applying those two fixes you added to my branch.

This is what it looks like without them: https://imgur.com/a/a8kmzvO

This is what it looks like with them: https://imgur.com/a/KGf3ijX

It's late now, so I'm going to have to look into that code individually later. You said the first one fixed your prompt issue, but was there some issue the second one was fixing?

ingwarsw commented 3 years ago

@warmstarter First one is for sure needed..

Second one was not fixing anything for me.. If its breaking something probably its ok to remove it..

warmstarter commented 3 years ago

I tried with just the first patch, and that's what's causing the breakage. Oddly, though, it's not a general utf8 breakage as I thought. I'm going to have to look into why this is hapenning in certain places and not others.

ingwarsw commented 3 years ago

@warmstarter But first change (moving handle_socket_input_queue_lines outside #if WIDECHAR) cannot break anything when compiled with widechar.

Code with widechar is exactly same as before..

Maybe you meant that other part???