Skip to content

fix randomize settings#38

Open
telephon wants to merge 1 commit into
mainfrom
fix-randomize-settings
Open

fix randomize settings#38
telephon wants to merge 1 commit into
mainfrom
fix-randomize-settings

Conversation

@telephon

@telephon telephon commented Sep 1, 2025

Copy link
Copy Markdown
Contributor

This has been around for a long time, @adcxyz could you check?

@adcxyz

adcxyz commented Sep 2, 2025

Copy link
Copy Markdown
Contributor

As is, this seems to revert a lot of newer code - maybe it needs to be rebased first?
I am not sure what the specific section that is supposed to fix randomizing.
There is a newer fix to one conceptual error in setRand, which this PR reverts
(this one keeps arrays as param values intact):

	-	proxy.set(*this.randSet(rand, startSet, except, seed).flatten(1));
	+      proxy.set(*this.randSet(rand, startSet, except, seed).flat);	

@adcxyz

adcxyz commented Sep 3, 2025

Copy link
Copy Markdown
Contributor

I looked again, and I don't find any functional improvement here,
but lots of unintended reverts.
@telephon, if you like, please rebase and check if you see any useful changes remaining.
If not, we can just drop this one.

@telephon

telephon commented Sep 8, 2025

Copy link
Copy Markdown
Contributor Author

I don't see – onto what branch you want me to rebase? It is on main already.

@adcxyz

adcxyz commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Can you remember what the specific fix here was?
I cannot see what it could be, there is so much code that reverts later changes.

@telephon

Copy link
Copy Markdown
Contributor Author

I can*t remember what it was.

maybe this gives a hint?
58975cf#diff-5cb1ad42ca7bf50ce2b66737ea4c3c1275535c8f9624136787340ea65c43d303R94

Unfortunately, there is no issue connected.

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