Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added showing patch notes to the main menu #4101

Merged
merged 25 commits into from
Feb 8, 2023

Conversation

hhyyrylainen
Copy link
Member

@hhyyrylainen hhyyrylainen commented Feb 3, 2023

Brief Description of What This PR Does

Added displaying patch notes to the main menu on update and added a button in options to view patch notes anytime. There's an option to disable the feature. Also contains small tweaks (and one fix) for the thrive news feed.

Related Issues

closes #3868
closes #3880

Progress Checklist

Note: before starting this checklist the PR should be marked as non-draft.

  • PR author has checked that this PR works as intended and doesn't
    break existing features:
    https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
    (this is important as to not waste the time of Thrive team
    members reviewing this PR)
  • Initial code review passed (this and further items should not be checked by the PR author)
  • Functionality is confirmed working by another person (see above checklist link)
  • Final code review is passed and code conforms to the
    styleguide.

Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.

@hhyyrylainen hhyyrylainen marked this pull request as ready for review February 7, 2023 12:57
@hhyyrylainen hhyyrylainen requested review from 84634E1A607A and a team February 7, 2023 12:57
@Gameboy555r
Copy link

i took a look into this and, i dunno if its just for me but i works fine.

@84634E1A607A
Copy link
Contributor

84634E1A607A commented Feb 8, 2023

Many patch note links lost its left parenthesis
image

APPEND: When it exists in the json file.

Copy link
Contributor

@84634E1A607A 84634E1A607A left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I found a few bugs. Other than that (and the UI) I have nothing to comment on. I'm not a steam user so I'm not sure what it looks like when using steam and a packaged built.

Also I wonder if it is preferable to move all those news conversions to our web server, and make the generation script to parse all those notes to its final bbcode state. It's no point to download a number of web pages and spend time parsing them.

@hhyyrylainen
Copy link
Member Author

Many patch note links lost its left parenthesis

Ah, I'm accidentally losing one character in the regex replace call, I'll fix that to include the character that was accidentally removed.

Also I wonder if it is preferable to move all those news conversions to our web server, and make the generation script to parse all those notes to its final bbcode state. It's no point to download a number of web pages and spend time parsing them.

I guess that is possible. Most people use web browsers that try to understand html to display it. The launcher does the exact same thing, it parses HTML. If this was changed in a sensible way, then there'd need to be multiple variants of the data for Thrive and launcher separately. Also going that far, the markdown to bbcode conversion could also be done ahead of time.

That is possible but adds the problem that the link clickable colour will be set by the server so if someone wants to change the link colour in Thrive they need to also modify the server software and re-process all the already generated patch notes data.

@84634E1A607A
Copy link
Contributor

Link-clickable color can be set as a placeholder (For example #LINK_COLOR) and use string replacement which is faster. And I'm actually saying that a script can be added at the server side to calculate and cache feeds data, which will reduce overall calc time, reduce calc duplication and reduce network packet length.

@hhyyrylainen
Copy link
Member Author

Opened follow up issues: #4113
and #4114

For good measure I also opened an issue for the launcher: Revolutionary-Games/Thrive-Launcher#259

@hhyyrylainen hhyyrylainen merged commit 28de2e8 into master Feb 8, 2023
@hhyyrylainen hhyyrylainen deleted the 3868_patch_notes_in_game branch February 8, 2023 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

The main menu should show latest patch notes after version update
3 participants