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

Fix memory leak and (kinda) cleanup code? #776

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ASpoonPlaysGames
Copy link
Contributor

To try and preserve behaviour I used do while which I don't think ended up making the code much more readable... ah well, fixed a memory leak at least.

Testing:

  • Not really certain, but I think a good number of hooks would break if this code was broken, so uhh launch the game and play a match or something I guess

@ASpoonPlaysGames ASpoonPlaysGames added needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code labels Aug 25, 2024
@F1F7Y
Copy link
Member

F1F7Y commented Aug 26, 2024

Considering we want to nuke AUTOHOOK is there any use in potentially breaking it just for it to look slightly nicer?

@GeckoEidechse
Copy link
Member

Tried testing on Linux but it failed to launch and in the process my Wine prefix got polluted to the point where EA would crash. I'm not sure if this is a Linux skill issue or an effect of this PR.

Either way, if someone could test this on Windows in the meantime to LMK whether it works or not, that'd be great <3

@GeckoEidechse
Copy link
Member

@RoyalBlue1 said she agrees with @F1F7Y on this.

Copy link

@NachosChipeados NachosChipeados left a comment

Choose a reason for hiding this comment

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

Works fine for me. Played 2 full matches: one in Northstar and one in Vanilla+ with no crashes. Tested on Windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs code review Changes from PR still need to be reviewed in code needs testing Changes from the PR still need to be tested
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

5 participants