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

Move config to lazy_lock and into own crate #68

Merged
merged 12 commits into from
Sep 1, 2024

Conversation

DaniD3v
Copy link
Contributor

@DaniD3v DaniD3v commented Aug 31, 2024

Please take a look at the LazyLock docs docs first, because they're a relatively new feature and a few people in the discord confused it with something else.

TL;DR it's basically lazy_static but in std.


Moving the config to LazyLock has a few advantages:

  • easy access from everywhere. No need to pass the config through some functions.
    Just write BASIC_CONFIG.<config option> and you're good.
  • It's static so you can take &'static borrows
  • The performance of a static is probably better than passing references everywhere
  • There is barely any overhead on LazyLock once the config is initialized

Downsides:

  • We need to move some stuff into pumpkin-core if we want to access it from the config and whatever crate we're modifying.
  • It's static so we can't modify the config variable.
  • MSRV

  • It's static so we can't modify the config variable.

This was one of the main points why people were LazyLock,
however I don't think that's a problem for this specific case of storing the config because

  • We can't write to the config anyways (Think immutable linux distros + user comments)
  • The config field should not be used to store data that is different from the data in the actual config.
  • I don't think automatically reloading the config on changes is a good idea,
    considering the servers startup time and emphasis on performance

This PR would make it significantly easier for me to write the superflat WorldGen config.

That's because I'd have to pass the config through ~4 methods to get it to the constructor of the worldgen +
I'd need to change the GeneratorInit trait to take a seed and the config.


All of the commits compile individually and were checked with

cargo clippy
cargo build
cargo check --release
cargo fmt --check

@Snowiiii
Copy link
Owner

Snowiiii commented Sep 1, 2024

thank you @DaniD3v

@Snowiiii Snowiiii merged commit f6151df into Snowiiii:master Sep 1, 2024
5 checks passed
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.

2 participants