Skip to content

Add UI Notification for Invalid Mod Installs#597

Draft
uniboi wants to merge 26 commits into
R2Northstar:mainfrom
uniboi:invalid-mod-installs
Draft

Add UI Notification for Invalid Mod Installs#597
uniboi wants to merge 26 commits into
R2Northstar:mainfrom
uniboi:invalid-mod-installs

Conversation

@uniboi

@uniboi uniboi commented Feb 26, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

F1F7Y
F1F7Y reviewed Feb 26, 2023
View reviewed changes

@F1F7Y F1F7Y left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looks good, might be worth putting the invalid mods at the start of the list so that users cant miss them

Comment thread Northstar.Client/mod/resource/northstar_client_localisation_english.txt
@BobTheBob9

BobTheBob9 commented Feb 26, 2023

Copy link
Copy Markdown
Member

haven't tested, but ui looks great!

Alystrasz
Alystrasz requested changes Feb 26, 2023
View reviewed changes

@Alystrasz Alystrasz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looks good, works along R2Northstar/NorthstarLauncher#415 with one or several fucked mods.
I however have some requests regarding UI.

Comment thread Northstar.Client/mod/scripts/vscripts/ui/menu_ns_modmenu.nut
Comment thread Northstar.Client/mod/resource/northstar_client_localisation_english.txt Outdated
Comment thread Northstar.Client/mod/resource/ui/menus/panels/toast_popup.res
Comment thread Northstar.Client/mod/scripts/vscripts/ui/panel_mainmenu.nut Outdated
@Alystrasz Alystrasz mentioned this pull request Feb 27, 2023
Draft
1 task
@uniboi

uniboi commented Feb 27, 2023

Copy link
Copy Markdown
Contributor Author

The compile check fails because some exposed function signatures have been corrected in R2Northstar/NorthstarLauncher#415.

@GeckoEidechse

GeckoEidechse commented Feb 27, 2023

Copy link
Copy Markdown
Member

The compile check fails because some exposed function signatures have been corrected in R2Northstar/NorthstarLauncher#415.

In that case you can already update the signatures in .github/nativefuncs.json. The mods CI technically does not depend on launcher code (but should obviously be kept in sync :P)

@uniboi

uniboi commented Feb 28, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@GeckoEidechse panel_mainmenu.nut is a new vanilla file and nativefuncs are updated

@F1F7Y F1F7Y added the waiting on changes by author Waiting on PR author to implement the suggested changes label Apr 20, 2023
@uniboi uniboi requested review from Alystrasz and F1F7Y May 7, 2023 17:21
@uniboi uniboi added needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code and removed waiting on changes by author Waiting on PR author to implement the suggested changes labels May 7, 2023
F1F7Y
F1F7Y approved these changes May 8, 2023
View reviewed changes

@F1F7Y F1F7Y left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looks good, tested with Alystrasz/NorthstarLauncher#4

Alystrasz
Alystrasz requested changes May 9, 2023
View reviewed changes

@Alystrasz Alystrasz left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looks good, confirmed working on Ubuntu 23.04.

image

I'd like to see mod.json file mentioned in error messages though.

Comment thread Northstar.Client/mod/resource/northstar_client_localisation_english.txt Outdated
ASpoonPlaysGames
ASpoonPlaysGames approved these changes May 18, 2023
View reviewed changes

@ASpoonPlaysGames ASpoonPlaysGames left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

image
works in testing, notification appears and all works fine.

Code looks good, moving mod browser and stuff to use structs instead of loads of native functions really helps clean up the code

Comment thread Northstar.Client/mod/resource/northstar_client_localisation_english.txt
@uniboi uniboi added almost ready to merge Apart from any small remaining other issues addressed by other labels, this would be ready to merge and removed needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code labels May 30, 2023
@uniboi uniboi added the needs testing Changes from the PR still need to be tested label Jul 14, 2023
@F1F7Y F1F7Y added the merge conflicts Blocked by merge conflicts, waiting on the author to resolve label Jul 26, 2023
@ASpoonPlaysGames

ASpoonPlaysGames commented Aug 25, 2023

Copy link
Copy Markdown
Contributor

@uniboi this is currently broken due to outside changes using native functions that are removed in this PR. (also merge conflicts)

@ASpoonPlaysGames ASpoonPlaysGames added waiting on changes by author Waiting on PR author to implement the suggested changes and removed almost ready to merge Apart from any small remaining other issues addressed by other labels, this would be ready to merge labels Aug 25, 2023
@uniboi

uniboi commented Aug 26, 2023

Copy link
Copy Markdown
Contributor Author

I'm not sure if this is even needed anymore. I haven't seen someone manually installing mods incorrectly in a while. Maybe most people are using mod managers now?

@dbyale

dbyale commented Aug 26, 2023

Copy link
Copy Markdown
Contributor

I'm not sure if this is even needed anymore. I haven't seen someone manually installing mods incorrectly in a while. Maybe most people are using mod managers now?

A vast amount of people dont use mod managers and screw it up

Plus, this extra info doesnt hurt

@ASpoonPlaysGames ASpoonPlaysGames added the depends on another PR Blocked until the PR it depends on is merged label Sep 2, 2023
@Alystrasz

Alystrasz commented Oct 6, 2023

Copy link
Copy Markdown
Member

@uniboi I think your branch is outdated, the feature does not work anymore

@GeckoEidechse

GeckoEidechse commented Oct 10, 2023

Copy link
Copy Markdown
Member

@uniboi there's currently some merge conflicts.

@GeckoEidechse

GeckoEidechse commented Nov 20, 2023

Copy link
Copy Markdown
Member

@uniboi bump

@GeckoEidechse

GeckoEidechse commented Mar 4, 2024

Copy link
Copy Markdown
Member

bump

@Alystrasz

Alystrasz commented Nov 7, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

@uniboi 'sup?

[EDIT]: Launcher counterpart PR (R2Northstar/NorthstarLauncher#415) is now up-to-date with main.

@Alystrasz

Alystrasz commented Feb 1, 2025

Copy link
Copy Markdown
Member

Moving the PR to draft while waiting for changes from authors.

@Alystrasz Alystrasz marked this pull request as draft February 1, 2025 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Alystrasz Alystrasz Alystrasz approved these changes
@F1F7Y F1F7Y F1F7Y approved these changes
@ASpoonPlaysGames ASpoonPlaysGames ASpoonPlaysGames approved these changes

Assignees

No one assigned

Labels

depends on another PR Blocked until the PR it depends on is merged merge conflicts Blocked by merge conflicts, waiting on the author to resolve needs testing Changes from the PR still need to be tested waiting on changes by author Waiting on PR author to implement the suggested changes

Projects

Northstar Roadmap
Status: No status

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@uniboi @BobTheBob9 @GeckoEidechse @ASpoonPlaysGames @dbyale @Alystrasz @F1F7Y