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

feat: add poolUsage(s)/assetCount commands #89

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Lierrmm
Copy link

@Lierrmm Lierrmm commented Feb 8, 2024

What does this PR do?

Adds 3 new commands that can be used for seeing the total number of xassets used.

  • poolUsages
  • poolUsage
  • assetCount

How does this PR change IW4x's behaviour?

Existing behavior does not change.

  • poolUsages

    • Lists the usage/total count of each xasset type
  • poolUsage <number>

    • Lists the usage/total count of the specified xasset type
  • assetCount

    • List the total amount of xassets used/total xasset limit
  • poolUsages
    image

  • poolUsage <number>
    image

  • assetCount
    image

Anything else we should know?

N/A

Did you check all the boxes?

  • Focus on a single fix or feature; remove any unrelated formatting or code changes
  • Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
  • Follow our coding conventions
  • Minimize the number of commits

@@ -689,5 +698,55 @@ namespace Components
Logger::Print("Waiting for database...\n");
while (!Game::Sys_IsDatabaseReady()) std::this_thread::sleep_for(100ms);
});

Command::Add("poolUsages", []()
Copy link
Contributor

Choose a reason for hiding this comment

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

If you could make those commands 3 functions instead of defining their body in the constructor of FastFiles that would be better (when I add functions to IW4x that's what I'm trying to do from now on).

@@ -565,6 +565,15 @@ namespace Components
return file;
}

void enum_assets(const Game::XAssetType type, const std::function<void(Game::XAssetHeader, void*)> callback, const bool includeOverride)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure that function is necessary (it's basically a forwarder to Game::DB_EnumXAsset, i don't feel like it does much more?) but asssuming it would be:

  • I think this belongs in AssetHandler not FastFiles
  • PascalCase for functions not snake_case please !
  • Add it to the class as a private function rather than defining it in the CPP

count++;
}, true);

Logger::Print(Utils::String::VA("%i %s: %i / %i\n", i, Game::g_assetNames[i], count, Game::g_poolSize[i]));
Copy link
Contributor

@diamante0018 diamante0018 Feb 14, 2024

Choose a reason for hiding this comment

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

bad usage of logger. change to logger::print("{} {} etc etc")

learn more about c++20 format here https://en.cppreference.com/w/cpp/utility/format

count++;
}, true);

Logger::Print(Utils::String::VA("%i %s: %i / %i\n", type, Game::g_assetNames[type], count, Game::g_poolSize[type]));
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

totalCount += Game::g_poolSize[i];
}

Logger::Print(Utils::String::VA("total assets: %i / %i\n", count, totalCount));
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor

@diamante0018 diamante0018 left a comment

Choose a reason for hiding this comment

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

I'm porting this code to a fork to test it out and there are several issues with it besides the several issues already pointed out

@@ -565,6 +565,15 @@ namespace Components
return file;
}

void enum_assets(const Game::XAssetType type, const std::function<void(Game::XAssetHeader, void*)> callback, const bool includeOverride)
Copy link
Contributor

Choose a reason for hiding this comment

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

CamelCase + defined function as static member of fastfiles

return;
}

const auto type = static_cast<Game::XAssetType>(std::atoi(params->get(1)));
Copy link
Contributor

Choose a reason for hiding this comment

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

lack of bounds checking i can enter a negative number in the console and this code will happily take it in

@@ -39,7 +39,7 @@ namespace Game
typedef void(*DB_EnumXAssets_t)(XAssetType type, void(*)(XAssetHeader, void*), void* userdata, bool overrides);
extern DB_EnumXAssets_t DB_EnumXAssets;

typedef void(*DB_EnumXAssets_Internal_t)(XAssetType type, void(*)(XAssetHeader, void*), void* userdata, bool overrides);
typedef void(*DB_EnumXAssets_Internal_t)(XAssetType type, void(*)(XAssetHeader, void*), const void* userdata, bool overrides);
Copy link
Contributor

Choose a reason for hiding this comment

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

modify your code to match the game's function definitions not the other way around

@Rackover
Copy link
Contributor

@Lierrmm Hello, any news? 😄

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