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

Make RTP warnings more noticeable #1630

Merged
merged 3 commits into from Feb 26, 2019

Conversation

Projects
None yet
3 participants
@Ghabry
Copy link
Member

Ghabry commented Feb 10, 2019

By telling the user everytime a RTP asset is missing. Sound and Music is also a visual warning, because we got already many reports that our Player fails to play sound...
Additionally when the RTP is not installed and the game never uses the RTP (but has no fullpackageflag=1) the user won't receive this annoying warning on startup.

Is kinda-RFC, maybe you have a better idea.

Also pretty sure I fixed a bug because search_paths.clear(); was not called when the game loaded has FullPackageFlag=1

a-fs8

Fixes #1274.

@Ghabry Ghabry added the FileFinder label Feb 10, 2019

@Ghabry Ghabry force-pushed the Ghabry:rtp-warnings branch from 091987b to 4dc081d Feb 10, 2019

@fdelapena
Copy link
Contributor

fdelapena left a comment

Sounds like a great solution for the Android issue. 👍

@Ghabry Ghabry force-pushed the Ghabry:rtp-warnings branch from 4dc081d to 184bf5c Feb 10, 2019

@carstene1ns

This comment has been minimized.

Copy link
Member

carstene1ns commented Feb 10, 2019

Btw. without #1274, this will not display anything when I have added an empty RTP dir. 😁

@Ghabry

This comment has been minimized.

Copy link
Member Author

Ghabry commented Feb 10, 2019

hmm, good point. I will issue a different warning when a RTP is found but incomplete.

@Ghabry Ghabry force-pushed the Ghabry:rtp-warnings branch from 184bf5c to 95811d9 Feb 10, 2019

Show resolved Hide resolved src/filefinder.cpp Outdated

@fdelapena fdelapena added this to the 0.6.0 milestone Feb 12, 2019

Show resolved Hide resolved src/filefinder.cpp Outdated

@fdelapena fdelapena added the UX label Feb 20, 2019

@carstene1ns carstene1ns force-pushed the Ghabry:rtp-warnings branch from 95811d9 to e0538bf Feb 22, 2019

@carstene1ns

This comment has been minimized.

Copy link
Member

carstene1ns commented Feb 22, 2019

There is btw. still a bug when you use the gamebrowser and load a game with FullPackageFlag=1 and then one without.

@carstene1ns carstene1ns force-pushed the Ghabry:rtp-warnings branch from 45e43ac to 972297d Feb 22, 2019

@carstene1ns

This comment has been minimized.

Copy link
Member

carstene1ns commented Feb 22, 2019

The mentioned bug has been fixed. Also added additional logic to make Player still use RTP like RPG_RT when FullPackageFlag=1 has been set (as mentioned by Cherry). Please review.

@fdelapena
Copy link
Contributor

fdelapena left a comment

Code reviewed, not tested the binary yet. Looking good so far 👍. I forgot some stuff in the first approval now indicated.

Show resolved Hide resolved resources/easyrpg-player.6.adoc Outdated
Show resolved Hide resolved src/filefinder.cpp Outdated
Show resolved Hide resolved src/filefinder.cpp
Show resolved Hide resolved src/player.cpp Outdated
name == rtp_name ? "!" : rtp_name.c_str());
if (!disable_rtp_warnings && !rtp_name.empty()) {
std::string msg = "Cannot find: %s/%s. " + std::string(search_paths.empty() ?
"Install RTP %d to resolve this warning." : "RTP %d was probably not installed correctly.");

This comment has been minimized.

@fdelapena

fdelapena Feb 23, 2019

Contributor

Debatable: RTP %d or %d RTP?

Show resolved Hide resolved src/filefinder.cpp Outdated

@fdelapena fdelapena changed the title Make RTP warnings more noticable Make RTP warnings more noticeable Feb 23, 2019

@Ghabry Ghabry requested a review from carstene1ns Feb 23, 2019

@carstene1ns carstene1ns force-pushed the Ghabry:rtp-warnings branch from 972297d to 43132d1 Feb 23, 2019

Implement the same RTP logic RPG_RT uses:
- Try to load RTP paths always
- Games may still try to use RTP even with FullPackageFlag=1
Notify the user about games that are broken, because of this
Provide an option to completely disable all RTP support
(Related: #1274)

@Ghabry Ghabry force-pushed the Ghabry:rtp-warnings branch from 43132d1 to 75eff1b Feb 26, 2019

@Ghabry

This comment has been minimized.

Copy link
Member Author

Ghabry commented Feb 26, 2019

The check for Sound and Music was incorrect in the "rtp is disabled" path.

LGTM 👍

@carstene1ns carstene1ns merged commit a37f361 into EasyRPG:master Feb 26, 2019

7 checks passed

Android (armeabi-v7a) Build finished.
Details
GNU/Linux Build finished.
Details
OSX Build finished.
Details
Wii (SDL1) Build finished.
Details
Windows (x64) Build finished.
Details
Windows (x86) Build finished.
Details
web Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.