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

Core: set consistent server defaults #2566

Merged
merged 1 commit into from
Jan 14, 2024

Conversation

Berserker66
Copy link
Member

What is this fixing or adding?

set consistent server defaults
Some reasonings:
In general these should be reasonable defaults someone can just start a game with, and the other options should offer customization from there.

items plando allows modifying of other worlds, the others are world-local. It should therefore be opt in, preferably after the host has read the plando guide to know what to look out for.

Collect and Release on Auto, teaches that the systems exist, can be customized if changes are desired and is a reasonable default; as well as a default encountered in say The Big Async.

Moved Items plando visually up, as it has the biggest impact and support.

How was this tested?

I only tested the webhost changes

If this makes graphical changes, please attach screenshots.

image

@alwaysintreble alwaysintreble added is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. affects: core Issues/PRs that touch core and may need additional validation. affects: webhost Issues/PRs that touch webhost and may need additional validation. labels Dec 5, 2023
@t3hf1gm3nt
Copy link
Collaborator

would this change (specifically the plando part) mean that issue #1943 would finally be resolved?
if so, I approve of this change
if not, can we make it so it resolves said issue at the same time?

@ThePhar ThePhar added is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. is: maintenance Regular updates to requirements and utilities that do not fix bugs or change/add features. and removed is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. labels Dec 5, 2023
@black-sliver
Copy link
Member

fixes #1943

Plando and remaining lgtm. Did we ever get the majority on the auto-collect/release? Or is this a bdfl veto? :D

@Berserker66
Copy link
Member Author

There was supposed to be a vote?

@black-sliver
Copy link
Member

black-sliver commented Dec 28, 2023

The "external" asyncs i play in always use manual collect/release to avoid spamming (e.g. alttp), so that change may be unexpected for some people. (Vote wouldn't hurt, I guess)

In addition to that, if host.yaml already exists, the local behaviour will be different than the webhost behaviour - that is for all existing installations that upgrade.

@t3hf1gm3nt
Copy link
Collaborator

And that's why I brought up issue #1943 in my previous comment.
I personally don't care either way what the defaults are for release/collect. I just want them to be consistent between the webhost and local installs

Copy link
Collaborator

@Exempt-Medic Exempt-Medic left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@black-sliver black-sliver left a comment

Choose a reason for hiding this comment

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

Unless you want to make a poll for it, should probably put it in the release notes that the default for release/collect has changed.

@black-sliver
Copy link
Member

Oh, I forgot: I still think reversing the roles for release and collect would be the better Alternative than auto-release.

@Berserker66 Berserker66 merged commit 6ac3d5c into main Jan 14, 2024
21 checks passed
@Berserker66 Berserker66 deleted the core_consistent_server_defaults branch January 14, 2024 20:24
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. affects: webhost Issues/PRs that touch webhost and may need additional validation. is: maintenance Regular updates to requirements and utilities that do not fix bugs or change/add features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants