-
Notifications
You must be signed in to change notification settings - Fork 735
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
LADX: Full Entrance Rando #1613
Conversation
Closing for now, but will be back |
We played a dozen of sessions with this merged and tested pretty much all ER options. Everything seems fine 👍. |
Bump on this, just played a pretty 17 player LADX only async where half of the players or so had this on, no issues |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test gens succeeded, both with maxed out options and random options. 100 person multiworld test with all random options succeeded, but took a bit longer than I'd like. Unsure if it is because of ER or if it was always like that. I tested with accessibility locations and no locations showed as inaccessible.
I only reviewed init, options, and locations (the main apworld files), and did test gens. I did not review anything in the LADXR folder. I did not test the client.
@@ -1,13 +1,18 @@ | |||
import binascii | |||
import os | |||
import copy | |||
import itertools | |||
from .Locations import connector_info |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Combine this into the other .Locations import
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"this" being the from .Locations import connector_info
line. Forgot it shows like 5 lines instead of just the highlighted one.
import copy | ||
import itertools | ||
from .Locations import connector_info | ||
from .LADXR.entranceInfo import ENTRANCE_INFO, entrances_by_type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably move this down near the rest of the from .LADXR imports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"this" being the from LADXR.entranceInfo
import. Forgot it shows like 5 lines instead of just the highlighted one.
indoor_pools = {} | ||
entrance_pool_mapping = {} | ||
|
||
random = self.multiworld.per_slot_randoms[self.player] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
random = self.multiworld.per_slot_randoms[self.player] | |
random = self.random |
per_slot_randoms is deprecated, use self.random instead
Would recommend also making this change in other spots that use either per_slot_randoms or self.multiworld.random
Zig is no longer actively maintaining LADX. I'm still willing to merge LADX PRs, but only with a larger volume of peer reviews. Also, someone needs to sort out the conflicts, obviously |
Unless Zig wants to continue with this PR, someone else would need to reopen it in their stead (with permission of course) |
Zig gave me permission to take this PR and make a more merge-ready version. I'll close this after I've put up the new PR |
What is this fixing or adding?
Adds full ER to LADX. There's a few places where code isn't as efficient as I'd like, but I don't think it will impact perf on asyncs or anything (though open to someone testing that!)
How was this tested?
Multiple players have tested
If this makes graphical changes, please attach screenshots.
No