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

Use Local Path Command Line Option #1723

Closed
wants to merge 5 commits into from

Conversation

slackstone
Copy link
Member

Directory Options:
--use-local-path

Sets the default path precedence for loading local game assets.

Add-on assets contained in the local SuperTux/addon home directory, that also match an expected game asset path, will take precedence.

This feature allows game hackers the ability to load their own game assets and add-on files.

Example of addon home directory path, found on most Linux platforms:
~/.local/share/supertux2/addons

Copy link
Member

@Semphriss Semphriss left a comment

Choose a reason for hiding this comment

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

Mostly just indent fixes. Thanks for the PR!

(I didn't test the code yet, Ima do that in a while but for the moment, if something I said seems to break the code, leave it there and lmk)

Comment on lines 453 to 454
log_warning << "Could not add " << addon.get_install_filename() << " to search path: "
<< PHYSFS_getLastErrorCode() << std::endl;
Copy link
Member

Choose a reason for hiding this comment

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

Add extra indent

@slackstone slackstone requested a review from Semphriss April 15, 2021 22:59
Copy link
Member

@Semphriss Semphriss left a comment

Choose a reason for hiding this comment

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

Some more indentation problems, wrote the exact steps to fix or posted code snippets to make it easier

Comment on lines +455 to +456
}
else
Copy link
Member

Choose a reason for hiding this comment

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

One space too much in front of those two lines

Comment on lines +459 to +461
{
PHYSFS_enumerate(addon.get_id().c_str(), add_to_dictionary_path, nullptr);
}
Copy link
Member

Choose a reason for hiding this comment

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

Two spaces too much in front of those three lines

Comment on lines +466 to +478
{
if (PHYSFS_mount(addon.get_install_filename().c_str(), mountpoint.c_str(), 1) == 0)
{
log_warning << "Could not add " << addon.get_install_filename() << " to search path: "
<< PHYSFS_getLastErrorCode() << std::endl;
}
else
{
if (addon.get_type() == Addon::LANGUAGEPACK)
{
PHYSFS_enumerate(addon.get_id().c_str(), add_to_dictionary_path, nullptr);
}
addon.set_enabled(true);
Copy link
Member

Choose a reason for hiding this comment

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

    {
      if (PHYSFS_mount(addon.get_install_filename().c_str(), mountpoint.c_str(), 1) == 0)
      {
        log_warning << "Could not add " << addon.get_install_filename() << " to search path: "
                    << PHYSFS_getLastErrorCode() << std::endl;
      }
      else
      {
        if (addon.get_type() == Addon::LANGUAGEPACK)
        {
          PHYSFS_enumerate(addon.get_id().c_str(), add_to_dictionary_path, nullptr);
        }

Comment on lines +545 to +563
for (auto& addon : m_installed_addons)
{
if (is_old_enabled_addon(addon))
{
log_warning << "Could not add " << addon->get_install_filename() << " to search path: "
<< PHYSFS_getLastErrorCode() << std::endl;
if (g_config->use_local_path)
{
if (PHYSFS_mount(addon->get_install_filename().c_str(), mountpoint.c_str(), 0) == 0)
{
log_warning << "Could not add " << addon->get_install_filename() << " to search path: "
<< PHYSFS_getLastErrorCode() << std::endl;
}
}
else
{
if (PHYSFS_mount(addon->get_install_filename().c_str(), mountpoint.c_str(), 1) == 0)
{
log_warning << "Could not add " << addon->get_install_filename() << " to search path: "
<< PHYSFS_getLastErrorCode() << std::endl;
}
Copy link
Member

Choose a reason for hiding this comment

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

  for (auto& addon : m_installed_addons) 
  {
    if (is_old_enabled_addon(addon)) 
    {
      if (g_config->use_local_path) 
      {
        if (PHYSFS_mount(addon->get_install_filename().c_str(), mountpoint.c_str(), 0) == 0)
        {
          log_warning << "Could not add " << addon->get_install_filename() << " to search path: "
                      << PHYSFS_getLastErrorCode() << std::endl;
        }
      }
      else
      {
        if (PHYSFS_mount(addon->get_install_filename().c_str(), mountpoint.c_str(), 1) == 0)
        {
          log_warning << "Could not add " << addon->get_install_filename() << " to search path: "
                      << PHYSFS_getLastErrorCode() << std::endl;
        }

@Grumbel
Copy link
Member

Grumbel commented Apr 21, 2021

Is that option really necessary? Can't you just make that the default behavior and users that don't want to use it just don't copy addons there? There is already the option to enable/disable individual addons from within the game.

Either way, it would need a better name, --use-local-path really doesn't tell me much.

@slackstone
Copy link
Member Author

slackstone commented Apr 22, 2021 via email

@Semphriss
Copy link
Member

I'll be closing this as Tobbi's #1835 offers the best solution.

@Semphriss Semphriss closed this Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants