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

reshade: Summarise code-section of ReshadeUniform::{PingPongUniform,RandomUniform}() #1289

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

tcoyvwac
Copy link

@tcoyvwac tcoyvwac commented May 8, 2024

Hi there! 😄

Following the positive response of PR:#998, and a followup to PR:#1260, this PR condenses and summaries some common code-fragments in ReshadeUniform::{PingPongUniform,RandomUniform}'s constructors - ().

As usual, I kept the commit history for code-review purposes and ease however please feel free when this PR is complete, to condense / squash everything into one commit for git-history reasons if you like!

Please note:
There are additional points and questions discovered and raised in this PR. They are listed in a further comment ahead here.

Of course, happy to answer any questions and do my best explaining the design of the changeset.

Thanks and hope this helps! 😄

@tcoyvwac
Copy link
Author

tcoyvwac commented May 8, 2024

@misyltoad
Copy link
Collaborator

I will be honest, I find this much harder to read and rationalise about

@tcoyvwac
Copy link
Author

Hi @Joshua-Ashton, thanks for updating! Happy to make any changes! 😄

Sure thing! Is it the changeset / PR, or the condensed code, or the API calling the static free-function, or any other things, which is harder to read or rationalise about? Should the previous repeating code not be condensed at all?

There are quite a few lines of code which are repeating in this changeset, but condensing the code too much will make it harder to rationalise I agree, so looking for a happy medium with your guidance. 😄

@tcoyvwac tcoyvwac force-pushed the fix/reshade-condense-functions-with-lambdas-free-functions branch 4 times, most recently from 2b281b4 to 7f169cb Compare May 28, 2024 22:51
annotationAttributes for-loop check now handles an array of attributes
* processAttributes: local and parameter variable scoping + variable's
domain coupling (by using closures) reduced.

* reshade: inlined type_ternary parameter into IIFE
@tcoyvwac tcoyvwac force-pushed the fix/reshade-condense-functions-with-lambdas-free-functions branch from 7f169cb to f4bf38e Compare August 29, 2024 12:14
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