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

crate: Tidy up some out of place types #160

Merged
merged 10 commits into from
Jan 1, 2024

Conversation

Douile
Copy link
Collaborator

@Douile Douile commented Nov 22, 2023

My main aim here was to move types that are likely to be used by consumers of the library into the root scope of the library. I'm not sure if this is good practice but I thought it would be helpful:

TimeoutSettings and ExtraRequestSettings are now just in gamedig:: so users using generic query don't need to delve deep into the protocol type module.

use gamedig::{query_with_timeout_and_extra_settings, TimeoutSettings, ExtraRequestSettings, GAMES};

pub fn main() {
  let game = GAMES.get("teamfortess2").unwrap();
  let timeout = TimeoutSettings::new(/*...*/);
  let extra = ExtraRequestSettings::default();
  query_with_timeout_and_extra_settings(/*...*/);
}

I also moved the generic query functions, they should never have been in in the games/mod.rs file in the first place, although I'm not sure whether it would be better for them to be in games/query.rs or maybe even protocols/query.rs.

Where possible I added deprecation warnings in the old place, so the aliases can be removed in the future.

@github-actions github-actions bot added protocol This is something regarding a protocol game This is something regarding a game cli labels Nov 22, 2023
@CosminPerRam
Copy link
Member

CosminPerRam commented Dec 1, 2023

Hey, sorry for responding so late to this.
I do think that moving (or re-exporting) those types would be great for usage, but wouldn't just having

pub use protocols::types::{ExtraRequestSettings, TimeoutSettings};

in lib.rs be the same thing? Hope I don't miss on anything.

I also moved the generic query functions, they should never have been in in the games/mod.rs file in the first place, although I'm not sure whether it would be better for them to be in games/query.rs or maybe even protocols/query.rs.

Well said, I would say it would be better in games/query.rs.

@Douile
Copy link
Collaborator Author

Douile commented Dec 8, 2023

in lib.rs be the same thing? Hope I don't miss on anything.

Yes this would be same for library consumer, but I was also thinking it might be better for maintaining if the files are more organized (I'm not sure if the way I've done here is actually more organized).

Well said, I would say it would be better in games/query.rs.

Yeah, that makes more sense, especially since each game no longer has its own file.

@CosminPerRam
Copy link
Member

CosminPerRam commented Dec 10, 2023

(I'm not sure if the way I've done here is actually more organized)

I wouldn't think so.
They are primarily used for protocols, so I think they belong in the protocols folder.
But I do think that the types file is a bit too cluttered, I think it could benefit of some extraction, thoughts?

@Douile
Copy link
Collaborator Author

Douile commented Dec 27, 2023

So move from the games mod.rs to query.rs and types.rs. Yeah I agree the placement of ExtraRequest and TimeoutSettings isnt better. Im not sure where they are currently is the best place but I don't think where I moved them is better. So I'll leave that as is and just add re-exports in the root of the library to make consumption easier.

@github-actions github-actions bot removed the protocol This is something regarding a protocol label Dec 28, 2023
Douile and others added 2 commits January 1, 2024 22:11
This resolves conflicts in
- crates/cli/src/main.rs - TimeoutSettings/ExtraRequestSettings moved
- crates/lib/src/games/mod.rs - savage2 added but query functions were
  moved to new file
@CosminPerRam CosminPerRam merged commit bd3727d into gamedig:main Jan 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli game This is something regarding a game
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants