Closed thisis2838 closed 2 years ago
Sorry for the late reply, github emails are finnicky for me for some reason. Reviewing this soon.
Fixed mostly everything but the special split code.
Thanks, looking a lot better now! Can you rename the actual files disk to match the includes?
You mean the file names on GitHub?
ManualSplit
unused?This is probably annoying to hear but I'd rather have all the stuff added for Stanley Parable to be removed and added in another PR (it appears like a lot of this code is very hacky and needs to be reworked and It'd be nice to get all the other stuff merged in first)
Manual Split and Special Split are only used in TSP for now, since that game has 20 endings and most of them require very high precision. A lot of the code in TSP is hacky because of that + the fact that some endings currently have no real way of detecting such as for Song Ending, where the standard end point is when a specific Narrator voice line plays, or Stuck ending with the extremely hacky box position detection because I'd have to monitor a trigger_multiple. I'm up for removing TSP for now, I do think it is hacky but that's about all I could do. Would you mind showing me all the fields and names left to change so I can fix this?
Basically all the ones with underscores _trig_Index -> _trigIndex Looking pretty close to merge now :+1:
Done
I think this would work better for the timer reset related code:
Add a new field to GameSupport.cs named something like TimerResetSession
and games which require this will set this to true in their constructor
GameMemory.cs: Rename GameSupportOutBoundCalls
to _state
and make it non-static
GameMemory.cs: Make OnTimerReset
method which checks
_state.GameSupport.TimerResetSession, SignOnState.Full, HostState.Run
(check nulls)
then calls OnSessionStart
SourceSplit.cs: Use _gameMemory.OnTimerReset();
_onceFlag should now reset on timer reset, if I understand the purpose of the reset code correctly
I'd like to keep _onceFlag and _resetFlag separate. Some games and mods are timed with Chapter Select (Watching Paint Dry) or have a static start position (Black Mesa). In both cases since _onceFlag is reset on save / game load it's really easy to accidentally start the run again by a simple save/load. That's why I added _resetFlag, it's only reset on timer reset and stays the same throughout the run.
Can you do most of the changes I mentioned in my last message (except the TimerResetSession
stuff), but keep OnTimerReset
in GameSupport
like you have it, except remove the timerresetto
arg since it's always true? (OnTimerReset
in both GameMemory and GameSupport, GameMemory version with the checks (actually tbh not sure if the checks would make sense, do we want OnTimerReset to trigger while the game is still loading? you can decide)
ignore the review cancel message below
Can you do most of the changes I mentioned in my last message (except the
TimerResetSession
stuff), but keepOnTimerReset
inGameSupport
like you have it, except remove thetimerresetto
arg since it's always true? (OnTimerReset
in both GameMemory and GameSupport, GameMemory version with the checks (actually tbh not sure if the checks would make sense, do we want OnTimerReset to trigger while the game is still loading? you can decide)
still waiting on this btw
Imma get to work on it soon, probably gonna push all the changes to a new branch then start a new Pull Request from there though, the commit history on this one is trash lol
I was planning on squashing it, so don't worry about it
Imma leave timerresetto as is because setting it to the opposite on timer start will prevent the timer from resetting again after being started manually + see if I can make TSP's support less hacky
I need some thoughts on this: https://github.com/thisis2838/SourceSplit/blob/893c46dfa4fc312b0dd7c71024f3cf72ea236dc4/GameMemory.cs#L666-L667 I'm tired of using hacky methods like _resetFlag with runs that has their starts on map load and this'd make it much more clean and consistent Also can you review the rest of the other stuff like the newly rewritten TSP code if there's any problems?
I need some thoughts on this: https://github.com/thisis2838/SourceSplit/blob/893c46dfa4fc312b0dd7c71024f3cf72ea236dc4/GameMemory.cs#L666-L667
Looks fine. Added a few nitpicks. Please review the entire diff for var naming problems, I only pointed out a couple. We need to get the readme file updated with the new games and I could use a lengthy commit message to use on the squash of all the game additions / changes
Infra support was interesting as the game has a few quirks:
All this code is Infra-exclusive so it should have no effect on other games. Lemme know what to change
Let's try to get this merged before adding more games. Not a huge fan of the infra specific stuff but I'll take it for now.
Can you review the changes to Black Mesa? I think this is gonna be the last update Imma do to this in a while
been busy, will try to get this merged after holidays
You can see the newly-supported games here.
I've added a few functions such as finding an entity using coordinates instead of names and a "wrong" entity index number parameter for when there are multiple entities with the same name.
I've also added a special manual split timing method that:
This new splitting method is all contained within separate functions from the usual splitting code so it shouldn't mess with anything from before.