OnlineCop / kq-fork

Fork of KQ r910. Just for fun.
GNU General Public License v2.0
15 stars 9 forks source link

Improve implementation of quest items #137

Closed pedro-w closed 2 years ago

pedro-w commented 2 years ago

The Quest UI (accessible from the 'RETURN' menu) now shows a selectable list of item names on the left and corresponding text on the right. You can press ALT to switch to the right hand pane and scroll up/down if there is more text than will fit in the box.

However I removed all the item definitions from the scripts (they were just placeholder text) so currently it is not possible to see this view - hopefully I can start to add some items in before 1.0 is ready

Fixes #125

pedro-w commented 2 years ago

Screenshot 2022-08-11 at 13 32 48 FYI this is what it looks like

OnlineCop commented 2 years ago

After pulling this, I played through till I got to town2; I kept checking Quest but it never populated. I also searched for the text "Look for Derig in the grotto" but didn't find it.

Do I have to cheat those quests in, or do something specific to trigger the quests into my Quest list?

pedro-w commented 2 years ago

Do I have to cheat those quests in, or do something specific to trigger the quests into my Quest list?

I should have been clearer - in the top comment I said "However I removed all the item definitions from the scripts (they were just placeholder text) so currently it is not possible to see this view" - that's why I put the screenshot in just as an example of what it would look like. If you want to see it in action you could add some calls to add_quest_item in global.lua, function get_quest_info (line 550-ish)

pedro-w commented 2 years ago

I'm going to improve this, to give some feedback if there's a script error rather than just silently not working which is what happens now. So it's not ready for merge yet. Still haven't been able to reproduce your segfault, Z, maybe it's a windows thing?

z9484 commented 2 years ago

Yeah maybe its windows only. Heres where it crashes. Screenshot 2022-08-15 quest

pedro-w commented 2 years ago

it may be I made an incorrect assumption that just happened to work on Mac - I'll start up my old W10 laptop

pedro-w commented 2 years ago

Yow. yow

pedro-w commented 2 years ago

OK I think I have fixed it now - on Mac and Linux it's OK to form an out-of-range string iterator as long as you don't ever dereference it, but MSVC complains (at least in debug mode)

OnlineCop commented 2 years ago

This is what I see when running from Visual Studio 2022:

image

Would it look good if the circled chevrons were closer to the right edge of the text window?

This chevron is aligned with the right edge of its menu when there are enough quests to scroll to another page:

image

And this is entirely up to you: are there any other colors that we could use with the chevrons to make them stand out a little easier against the white edge of the menus?

OnlineCop commented 2 years ago

I haven't looked over the word-wrap code very carefully, but I threw together a bunch of edge cases to consider (punctuation, super long text, etc.

Super impressed.

image

Everything I worried would cause visual anomalies seem to be handled just fine.

Remember to clang-format the code before you merge 😉.

pedro-w commented 2 years ago

Would it look good if the circled chevrons were closer to the right edge of the text window?

Screenshot 2022-08-16 at 21 51 00

And this is entirely up to you: are there any other colors that we could use with the chevrons to make them stand out a little easier against the white edge of the menus?

They're graphics from misc.png, there is a 'gold' right chevron but not up & down. I suppose they could be added. At the moment it's using the graphics upptr and dnptr which are defined elsewhere. Definitely do-able.

pedro-w commented 2 years ago

Everything I worried would cause visual anomalies seem to be handled just fine.

I think it's ok, sometimes it looks like it could fit an extra word onto the line but if you count, it couldn't. The RHS looks raggedy because there's only 20 big characters per line. I guess it would be possible to stretch the spaces but that seemed too hard!

Remember to clang-format the code before you merge 😉.

Sure

OnlineCop commented 2 years ago

Is there a limit on the length of the quest keys ("New Quest 11" was a little too wide to fit in the left menu) that we need to keep in mind? We can avoid extra logic if you just throw a comment into global.lua warning the user to keep it under 11 characters.

OnlineCop commented 2 years ago

Do you think that this word wrap could also be incorporated into (and/or replace) KDraw::relay(), since that too seems to be doing its own word wrapping?

pedro-w commented 2 years ago

I wrote it with the idea that it could one day replace relay() but I didn't want to mess about with code that was working already. Note that relay() allows manual line breaks with the \n character but this new code doesn't (yet, it would be easy enough to add)

On Tue, 16 Aug 2022 at 23:22, OnlineCop @.***> wrote:

Do you think that this word wrap could also be incorporated into (and/or replace) KDraw::relay(), since that too seems to be doing its own word wrapping?

— Reply to this email directly, view it on GitHub https://github.com/OnlineCop/kq-fork/pull/137#issuecomment-1217229037, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFJFGZ4NO2BTEXXKTZ77YBLVZQIA7ANCNFSM56HAYERQ . You are receiving this because you authored the thread.Message ID: @.***>

pedro-w commented 2 years ago

Do you think that this word wrap could also be incorporated into (and/or replace) KDraw::relay(), since that too seems to be doing its own word wrapping?

I moved the layout code into KDraw, it made more sense there. I have prepared some code to replace KDraw::relay and it works OK but maybe will defer it until after 'PullZilla' (#142 and friends) :wink: