Skip to content

Commit

Permalink
linting fixups
Browse files Browse the repository at this point in the history
  • Loading branch information
apple1417 committed Dec 16, 2024
1 parent affe224 commit b077ad3
Show file tree
Hide file tree
Showing 27 changed files with 78 additions and 101 deletions.
8 changes: 8 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,14 @@ jobs:
with:
submodules: recursive

- name: Setup Python
uses: actions/setup-python@v5
with:
cache: pip

- name: Setup Pip
run: pip install pytest

- name: Run pyright
uses: jakebailey/pyright-action@v2

Expand Down
7 changes: 1 addition & 6 deletions command_extensions/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,15 +1,10 @@
file(GLOB_RECURSE sources CONFIGURE_DEPENDS "file_parser/*.cpp" "file_parser/*.h")

if(NOT DEFINED CE_FILE_PARSER_NATIVE_LINUX)
# By default, assume we're imported from the repo root, use pyunrealsdk to build
# By default, assume we're imported from the repo root, and just use pyunrealsdk to build

pyunrealsdk_add_module(file_parser ${sources})

# Slight hack: we don't actually rely on pyunrealsdk, but using it's helper function sets up
# everything for us, so just redefine the link libraries here to not rely on it
set_property(TARGET file_parser PROPERTY LINK_LIBRARIES)
target_link_libraries(file_parser PRIVATE explicit_python pybind11::module)

else()
# If `CE_FILE_PARSER_NATIVE_LINUX` is defined, instead treat this as a brand new cmake project
# This can be used to compile this as a native linux module - useful for tests or debugging on a
Expand Down
6 changes: 3 additions & 3 deletions command_extensions/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ enabled (and won't suddenly re-enable soon).

### Command Extensions v2
- Complete rewrite for v3 sdk.
- Added the `CE_NewCmd` command, for use in mod files that attempt to regsiter their own commands.
- Added the `CE_NewCmd` command, for use in mod files that attempt to register their own commands.
- Deprecated the `--suppress-exists` arg on `clone_bpd` and `clone`, existing uses are noop'd. While
developing, you can use the `clone_dbg_suppress_exists` command instead, which supresses the
developing, you can use the `clone_dbg_suppress_exists` command instead, which suppresses the
warning for all clone commands.
- Removed the `set_material` command.

Expand Down Expand Up @@ -71,4 +71,4 @@ enabled (and won't suddenly re-enable soon).
- Switched the demo mod file to "online" mode.

### Command Extensions v1.0
- Inital Release.
- Initial Release.
8 changes: 4 additions & 4 deletions command_extensions/Writing-Mods.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ of the original objects, dump them manually to check what their new names are.
## `clone_dbg_suppress_exists`
usage: `clone_dbg_suppress_exists [-h] {Enable,Disable}`

Supresses the 'object already exists' errors which may occur while cloning.
Suppresses the 'object already exists' errors which may occur while cloning.
Only intended for debug usage.

| positional arguments | |
Expand Down Expand Up @@ -181,7 +181,7 @@ usage: `load_package [-h] [--list] [package]`
Loads a package and all objects contained within it. This freezes the game as
it loads; it should be used sparingly. Supports using glob-style wildcards to
load up to 10 packages at once, though being explicit should still be
prefered.
preferred.

| positional arguments | |
| :------------------- | :-------------------------------------------------------------------------------------------- |
Expand Down Expand Up @@ -299,7 +299,7 @@ whitespace is ignored.

BLCMM files use a bit more involved parsing. Custom commands can be enabled and disabled by toggling
the category they're in. This requires a bit a workaround however, which might cause some
interesting behaviour. BLCMM unfortuantly only stores if individual set commands enabled. Our custom
interesting behaviour. BLCMM unfortunately only stores if individual set commands enabled. Our custom
commands are technically comments, so they don't get this, and categories themselves don't have a
specific enabled state either. Instead, we have to determine if a custom command is enabled by if
the regular set commands around it are.
Expand All @@ -318,7 +318,7 @@ considered to be enabled.
## Adding custom commands
Command Extensions is built on top of the command system in `mods_base`. After creating a command
normally, you must register it with command extensions in order to let it work from mod files. You
can do this either by manually callling `command_extensions.register` and
can do this either by manually calling `command_extensions.register` and
`command_extensions.deregister`, or you can use `command_extensions.autoregister` to do so
automatically when the command is enabled/disabled.

Expand Down
4 changes: 2 additions & 2 deletions command_extensions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ def execute_file(file_path: Path) -> None:
try:
exec(line[cmd_len:].lstrip(), py_globals) # noqa: S102
except Exception: # noqa: BLE001
logging.error("Error occured during 'py' command:")
logging.error("Error occurred during 'py' command:")
logging.error(line)
traceback.print_exc()

Expand All @@ -201,7 +201,7 @@ def execute_file(file_path: Path) -> None:
# To match pyunrealsdk, each pyexec gets a new empty of globals
exec(file.read(), {"__file__": str(path)}) # noqa: S102
except Exception: # noqa: BLE001
logging.error("Error occured during 'pyexec' command:")
logging.error("Error occurred during 'pyexec' command:")
logging.error(line)
traceback.print_exc()

Expand Down
2 changes: 1 addition & 1 deletion command_extensions/builtins/clone.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def clone(args: argparse.Namespace) -> None: # noqa: D103

@command(
description=(
"Supresses the 'object already exists' errors which may occur while cloning. Only intended"
"Suppresses the 'object already exists' errors which may occur while cloning. Only intended"
" for debug usage."
),
)
Expand Down
2 changes: 1 addition & 1 deletion command_extensions/builtins/load_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
description=(
"Loads a package and all objects contained within it. This freezes the game as it loads; it"
" should be used sparingly. Supports using glob-style wildcards to load up to 10 packages"
" at once, though being explicit should still be prefered."
" at once, though being explicit should still be preferred."
),
)
def load_package(args: argparse.Namespace) -> None: # noqa: D103
Expand Down
2 changes: 1 addition & 1 deletion command_extensions/builtins/pyb.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def pyb(args: argparse.Namespace) -> None: # noqa: D103
try:
exec(joined, {}) # noqa: S102
except Exception: # noqa: BLE001
logging.error("Error occured during 'pyb' command:")
logging.error("Error occurred during 'pyb' command:")
logging.error(joined)
if len(cached_lines) > 1:
logging.error("=" * 80)
Expand Down
2 changes: 1 addition & 1 deletion command_extensions/builtins/regen_balance.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ def gather_parts_lists(
Given a balance, work out what it's final parts list should be.
Args:
final_bal: The balance to gather tha parts lists of.
final_bal: The balance to gather the parts lists of.
item_type: What item type this balance is.
Returns:
A parts list dict, or None on error.
Expand Down
34 changes: 0 additions & 34 deletions command_extensions/file_parser/CMakeLists.txt

This file was deleted.

24 changes: 12 additions & 12 deletions command_extensions/file_parser/blcm_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,24 +69,24 @@ struct CommandBlock {
if (non_space == val.end()) {
return;
}
auto strat_end =
auto strategy_end =
std::find_if(non_space, val.end(), [](auto chr) { return std::isspace(chr); });

auto strategy_name = CaseInsensitiveStringView{non_space, strat_end};
auto strategy_name = CaseInsensitiveStringView{non_space, strategy_end};

static const constexpr CaseInsensitiveStringView strat_all = "All";
static const constexpr CaseInsensitiveStringView strat_any = "Any";
static const constexpr CaseInsensitiveStringView strat_force = "Force";
static const constexpr CaseInsensitiveStringView strat_next = "Next";
static const constexpr CaseInsensitiveStringView strategy_all = "All";
static const constexpr CaseInsensitiveStringView strategy_any = "Any";
static const constexpr CaseInsensitiveStringView strategy_force = "Force";
static const constexpr CaseInsensitiveStringView strategy_next = "Next";

EnableStrategy new_strategy{};
if (strategy_name == strat_all) {
if (strategy_name == strategy_all) {
new_strategy = EnableStrategy::ALL;
} else if (strategy_name == strat_any) {
} else if (strategy_name == strategy_any) {
new_strategy = EnableStrategy::ANY;
} else if (strategy_name == strat_force) {
} else if (strategy_name == strategy_force) {
new_strategy = EnableStrategy::FORCE;
} else if (strategy_name == strat_next) {
} else if (strategy_name == strategy_next) {
new_strategy = EnableStrategy::NEXT;
} else {
return;
Expand Down Expand Up @@ -162,7 +162,7 @@ struct CommandBlock {
};

/**
* @brief Recusively look through BLCMM categories and extract all enabled commands within them.
* @brief Recursively look through BLCMM categories and extract all enabled commands within them.
*
* @param category The XML category to look through.
* @param input_strategy The current enable strategy.
Expand Down Expand Up @@ -195,7 +195,7 @@ void handle_category(const pugi::xml_node& category,

static const constexpr CaseInsensitiveStringView code = "code";
if (child_name == code) {
auto is_enabled = blcm_preprocessor::in_comma_seperated_list(
auto is_enabled = blcm_preprocessor::in_comma_separated_list(
profile, child.attribute("profiles").as_string());

block.handle_standard_command(is_enabled);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ void process_tag_content(std::string_view line,
return;
}

std::string_view content{ittr, line.end()};
const std::string_view content{ittr, line.end()};

// Make sure the line ends with a valid closing tag
auto closing_tag_start = content.rfind("</");
Expand Down Expand Up @@ -206,7 +206,7 @@ bool preprocess_line(std::string_view line,
auto tag_start = line.find_first_of('<');
if (tag_start == std::string_view::npos) {
throw ParserError(
std::format("Failed to parse line (couldn't find inital tag):\n{}", line));
std::format("Failed to parse line (couldn't find initial tag):\n{}", line));
}

// Ignore the filtertool warning - which starts one char before the opening bracket
Expand All @@ -217,7 +217,7 @@ bool preprocess_line(std::string_view line,
auto tag_name_end = line.find_first_of("> \t", tag_start);
if (tag_name_end == std::string_view::npos) {
throw ParserError(
std::format("Failed to parse line (inital tag doesn't close):\n{}", line));
std::format("Failed to parse line (initial tag doesn't close):\n{}", line));
}

// Note this may include a leading `/` if looking at a closing tag
Expand Down Expand Up @@ -280,7 +280,7 @@ void preprocess(std::istream& blcmm_input, std::ostream& xml_output) {
}
}

bool in_comma_seperated_list(std::string_view value, std::string_view list) {
bool in_comma_separated_list(std::string_view value, std::string_view list) {
for (size_t entry_start = 0; entry_start < list.size();) {
auto entry_end = list.find_first_of(',', entry_start);
if (list.substr(entry_start, entry_end - entry_start) == value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ class ParserError : public std::runtime_error {
void preprocess(std::istream& blcmm_input, std::ostream& xml_output);

/**
* @brief Checks if a string is in a comma seperated list.
* @brief Checks if a string is in a comma separated list.
* @note Intended to be used to check if a command is active in the current profile.
*
* @param value The value to search for.
* @param list The comma seperated list to search through.
* @param list The comma separated list to search through.
* @return True if the value is found in the list, false otherwise.
*/
bool in_comma_seperated_list(std::string_view value, std::string_view list);
bool in_comma_separated_list(std::string_view value, std::string_view list);

} // namespace blcm_preprocessor

Expand Down
4 changes: 2 additions & 2 deletions command_extensions/file_parser/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ PYBIND11_MODULE(file_parser, mod) {

std::vector<CommandMatch> matches{};
if (line.rfind("<BLCMM", 0) == 0) {
matches = std::move(parse_blcmm_file(file));
matches = parse_blcmm_file(file);
} else {
matches = std::move(parse_file_line_by_line(file));
matches = parse_file_line_by_line(file);
}

std::vector<py::tuple> output;
Expand Down
33 changes: 15 additions & 18 deletions command_extensions/file_parser/matcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ bool CaseInsensitiveStringView::operator==(std::string_view str) const {

namespace {

// To match strings somewhat efficently we'll use a binary search - while not profiled I don't think
// we have enough strings and enough matching prefixes to make a full radix trie worth it.
// To match strings somewhat efficiently we'll use a binary search - while not profiled I don't
// think we have enough strings and enough matching prefixes to make a full radix trie worth it.

// For this we need to keep our list of commands sorted.
std::vector<CaseInsensitiveString> sorted_commands;
Expand All @@ -53,15 +53,13 @@ void update_commands(const std::vector<std::string_view>& commands) {
sorted_commands.clear();
sorted_commands.reserve(commands.size());

std::transform(commands.begin(), commands.end(), std::back_inserter(sorted_commands),
[](auto cmd) { return CaseInsensitiveString{cmd}; });

std::sort(sorted_commands.begin(), sorted_commands.end());
std::ranges::transform(commands, std::back_inserter(sorted_commands),
[](auto cmd) { return CaseInsensitiveString{cmd}; });
std::ranges::sort(sorted_commands);
}

void add_new_command(CaseInsensitiveStringView cmd) {
auto non_space =
std::find_if_not(cmd.begin(), cmd.end(), [](auto chr) { return std::isspace(chr); });
auto non_space = std::ranges::find_if_not(cmd, [](auto chr) { return std::isspace(chr); });
if (non_space == cmd.end()) {
return;
}
Expand All @@ -78,22 +76,21 @@ void add_new_command(CaseInsensitiveStringView cmd) {
}

CaseInsensitiveString cmd_name{non_space, cmd_name_end};
sorted_commands.insert(
std::lower_bound(sorted_commands.begin(), sorted_commands.end(), cmd_name),
std::move(cmd_name));
sorted_commands.insert(std::ranges::lower_bound(sorted_commands, cmd_name),
std::move(cmd_name));
}

#pragma endregion

std::pair<std::string_view, CommandMatch> try_match_command(std::string_view line) {
auto non_space =
std::find_if_not(line.begin(), line.end(), [](auto chr) { return std::isspace(chr); });
auto non_space = std::ranges::find_if_not(line, [](auto chr) { return std::isspace(chr); });
if (non_space == line.end()) {
return {};
}

auto cmd_end = std::find_if(non_space, line.end(), [](auto chr) { return std::isspace(chr); });

// NOLINTNEXTLINE(modernize-use-ranges)
if (!std::binary_search(sorted_commands.begin(), sorted_commands.end(),
CaseInsensitiveStringView{non_space, cmd_end})) {
return {};
Expand All @@ -105,11 +102,11 @@ std::pair<std::string_view, CommandMatch> try_match_command(std::string_view lin
std::string cmd_str{non_space, cmd_end};
std::string line_str{line};

CommandMatch match{py::reinterpret_steal<py::object>(PyUnicode_DecodeLocaleAndSize(
cmd_str.data(), (Py_ssize_t)cmd_str.size(), nullptr)),
py::reinterpret_steal<py::object>(PyUnicode_DecodeLocaleAndSize(
line_str.data(), (Py_ssize_t)line_str.size(), nullptr)),
cmd_end - line.begin()};
const CommandMatch match{py::reinterpret_steal<py::object>(PyUnicode_DecodeLocaleAndSize(
cmd_str.data(), (Py_ssize_t)cmd_str.size(), nullptr)),
py::reinterpret_steal<py::object>(PyUnicode_DecodeLocaleAndSize(
line_str.data(), (Py_ssize_t)line_str.size(), nullptr)),
cmd_end - line.begin()};

return std::make_pair(std::string_view{non_space, cmd_end}, match);
}
Expand Down
12 changes: 11 additions & 1 deletion command_extensions/file_parser/matcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,17 @@ void update_commands(const std::vector<std::string_view>& commands);
*/
void add_new_command(CaseInsensitiveStringView cmd);

struct CommandMatch {
// By default, pybind tries to compile with visibility hidden
// If we have default visibility in a type holding pybind objects as members, this may cause a
// warning, since our type has greater visibility than it's members
// This macro sets the right visibility
#if defined(__MINGW32__)
#define PY_OBJECT_VISIBILITY __attribute__((visibility("hidden")))
#else
#define PY_OBJECT_VISIBILITY
#endif

struct PY_OBJECT_VISIBILITY CommandMatch {
py::object py_cmd;
py::object line;
Py_ssize_t cmd_len;
Expand Down
1 change: 1 addition & 0 deletions command_extensions/file_parser/pch.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <filesystem>
#include <fstream>
#include <iterator>
#include <ranges>
#include <string>
#include <string_view>
#include <unordered_set>
Expand Down
Loading

0 comments on commit b077ad3

Please sign in to comment.