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

Games: Change Config read/write types from i32 to u32 to prevent negative values #25348

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

Conversation

ninadsachania
Copy link
Contributor

No description provided.

@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Nov 10, 2024
@BertalanD
Copy link
Member

Your commit message should explain what motivated this change/why this is necessary.

@ninadsachania
Copy link
Contributor Author

Your commit message should explain what motivated this change/why this is necessary.

Done. Motivation: All these values should never have negative values.

@ninadsachania ninadsachania changed the title Games: Change Config read/write types from i32 to u32 Games: Change Config read/write types from i32 to u32 to prevent negative values Nov 10, 2024
@@ -173,8 +173,8 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
chess_widget.set_time_control_increment(new_game_dialog->time_control_increment());

Config::write_bool("Games"sv, "Chess"sv, "UnlimitedTimeControl"sv, new_game_dialog->unlimited_time_control());
Config::write_i32("Games"sv, "Chess"sv, "TimeControlSeconds"sv, new_game_dialog->time_control_seconds());
Config::write_i32("Games"sv, "Chess"sv, "TimeControlIncrement"sv, new_game_dialog->time_control_increment());
Config::write_u32("Games"sv, "Chess"sv, "TimeControlSeconds"sv, new_game_dialog->time_control_seconds());
Copy link
Contributor

Choose a reason for hiding this comment

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

you are implicitly converting int to uint here. If you want to change this to u32, consider changing the time-related member variables.


Config::write_i32("Flood"sv, ""sv, "board_rows"sv, board_rows);
Config::write_i32("Flood"sv, ""sv, "board_columns"sv, board_columns);
Config::write_u32("Flood"sv, ""sv, "board_rows"sv, board_rows);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure size_t and unsigned int are the same? If not, I suggest changing types of these variables to unsigned int in all the files in the Flood directory. Otherwise, feel free to leave it as is.

@@ -25,8 +25,8 @@ WordGame::WordGame()
: m_clear_message_timer(Core::Timer::create_single_shot(5000, [this] { clear_message(); }))
{
read_words();
m_num_letters = Config::read_i32("MasterWord"sv, ""sv, "word_length"sv, 5);
m_max_guesses = Config::read_i32("MasterWord"sv, ""sv, "max_guesses"sv, 6);
m_num_letters = Config::read_u32("MasterWord"sv, ""sv, "word_length"sv, 5);
Copy link
Contributor

Choose a reason for hiding this comment

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

the same comment as in the Flood files applies here, too.

@@ -48,17 +48,17 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
TRY(Core::System::unveil("/res", "r"));
TRY(Core::System::unveil(nullptr, nullptr));

size_t board_size = Config::read_i32("2048"sv, ""sv, "board_size"sv, 4);
u32 target_tile = Config::read_i32("2048"sv, ""sv, "target_tile"sv, 2048);
size_t board_size = Config::read_u32("2048"sv, ""sv, "board_size"sv, 4);
Copy link
Contributor

Choose a reason for hiding this comment

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

change size_t to unsigned int?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👀 pr-needs-review PR needs review from a maintainer or community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants