Skip to content

Conversation

@Bagellll
Copy link
Contributor

@Bagellll Bagellll commented Sep 8, 2022

Hello yet again,

As per #93 I have fixed the initial issue with the interface returning incorrect values but tried to go and make it work best I could for real rather than storing useless data as a patch. I asked Jorrit from the Jolt repo what out of the parameters was possible to implement and did so (which ended up only being the velocity related parameters). There are also inline functions for each parameter just in case they're ever needed again.

@misyltoad
Copy link
Owner

Maybe it would be better to just provide a simple stub that returns the default values the regular VPhysics exposes...?

Do we know of anything that relies on calling SetPerformanceSettings and values actually changing in the system?

@Bagellll
Copy link
Contributor Author

Bagellll commented Sep 8, 2022

That is what I did initially just for compatibility reasons. I probably should've made the PR then instead, I'll edit it now if you want.

When it comes to things relying on SetPerformanceSettings, if the scope of this project is outside just GMod (which has an api call for setting these details, I imagine someone or a mod needing it), there may be another sourcesdk project that would've relied on it. I'm just speculating but it seems reasonable.

But again, I'll push a commit to revert the functionality if you'd like. No biggie.

@misyltoad
Copy link
Owner

We can keep the PR around, but I would make one that just stubs it out for now, and we can come back to it if anything ends up depending on it.

@misyltoad
Copy link
Owner

You should do it with the values from regular VPhysics if you do stub it out, to keep behaviour consistent.

@Bagellll
Copy link
Contributor Author

Bagellll commented Sep 8, 2022

Before I added anything special it behaved just as intended, the values are set by the mod. Will do though, making a new PR.

@misyltoad
Copy link
Owner

I want to tag 0.11 today, so I may beat you to adding a stub here. :b

@Bagellll
Copy link
Contributor Author

Bagellll commented Sep 9, 2022

You did ;-;

@Bagellll
Copy link
Contributor Author

That last commit was because I forgot to put the actual application of the settings behind the check too. Stylistic choice to guard at the beginning

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