Skip to content

Conversation

@FileEX
Copy link
Member

@FileEX FileEX commented May 9, 2025

This PR is the beginning of the refactoring I mentioned a few days ago. Until now, to include a specific enum in a file, the entire Common.h file had to be included. In many parts of the code, entire files are still being included just to use a single enum. In large projects (like mtasa-blue), placing enums in separate files and a dedicated directory is one of the best practices for improving project structure and management, significantly enhancing code readability and slightly improving compile times.

This PR only covers the Common.h file on the client side, but ultimately, almost all enums will be moved to individual files in upcoming PRs.

At the moment, there are many inelegant conversions using static_cast and others, but this is only a temporary solution to ensure the project builds successfully. This will be cleaned up and properly organized during the refactoring of those files.

This is only a change in the location of enums and the replacement of enum with enum class, but nevertheless, I performed tests on a local debug server and observed no bugs.

Changes:

  • Extracted enums into separate files in the shared/sdk/enums directory
  • Replaced enum with enum class
  • Removed unused enums
  • Removed Hungarian notation from enum names

@blow1d

This comment was marked as spam.

@tederis
Copy link
Member

tederis commented May 13, 2025

This PR brings many scroped enumerations which is good, in general. But it compels us to use cumbersome constructions like static_cast<std::size_t>(VehicleDummies::VEHICLE_DUMMY_COUNT)>. I don't think that we should write the code this way. Instead you can combine unscoped enums and namespaces. For example:

namespace VehicleDummies
{
    enum Enum
    {
        LIGHT_FRONT_MAIN = 0,
        ...
    }
}

And then you can use VehicleDummies::VEHICLE_DUMMY_COUNT without any casts. When you need to access the enum type directly, it will be just VehicleDummies::Enum.

@FileEX
Copy link
Member Author

FileEX commented May 13, 2025

This PR brings many scroped enumerations which is good, in general. But it compels us to use cumbersome constructions like static_cast<std::size_t>(VehicleDummies::VEHICLE_DUMMY_COUNT)>. I don't think that we should write the code this way. Instead you can combine unscoped enums and namespaces. For example:

namespace VehicleDummies
{
    enum Enum
    {
        LIGHT_FRONT_MAIN = 0,
        ...
    }
}

And then you can use VehicleDummies::VEHICLE_DUMMY_COUNT without any casts. When you need to access the enum type directly, it will be just VehicleDummies::Enum.

Yes, you're absolutely right, but as I mentioned

At the moment, there are many inelegant conversions using static_cast and others, but this is only a temporary solution to ensure the project builds successfully. This will be cleaned up and properly organized during the refactoring of those files.

This is just a temporary solution to make the project compile. The main goal of this PR is to move the enums to separate files, and their structure will still change later on

@tederis
Copy link
Member

tederis commented May 13, 2025

This is just a temporary solution to make the project compile. The main goal of this PR is to move the enums to separate files, and their structure will still change later on

I see. But is there any special reason to do this work twice? Isn't it better to do it at once?

@FileEX
Copy link
Member Author

FileEX commented May 13, 2025

This is just a temporary solution to make the project compile. The main goal of this PR is to move the enums to separate files, and their structure will still change later on

I see. But is there any special reason to do this work twice? Isn't it better to do it at once?

I think it's fine for now, because ultimately these files will be refactored anyway, and when I get to each specific file, I'll make all the necessary changes at that time

@tederis
Copy link
Member

tederis commented May 13, 2025

I think it's fine for now, because ultimately these files will be refactored anyway, and when I get to each specific file, I'll make all the necessary changes at that time

Okay. I'm not against it, just wanted to understand the order of things.

@tederis tederis requested a review from sbx320 May 18, 2025 12:29
@sbx320 sbx320 merged commit 1e56571 into multitheftauto:master May 18, 2025
6 checks passed
MTABot pushed a commit that referenced this pull request May 18, 2025
1e56571 Cleaning up client Common.h and moving enums to separate files (#4215)
@FileEX FileEX deleted the refactor/enums_commonh branch May 22, 2025 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants