StudioInfinity/dyna3
2
0
Fork 1
Code Issues 56 Pull requests 1 Projects 3 Releases 4 Wiki Activity Actions

simplify-names #111

Merged
glen merged 3 commits from Vectornaut/dyna3:simplify-names into main 2025-08-07 23:24:09 +00:00
Conversation 1 Commits 3 Files changed 7 +64 -64
Vectornaut commented 2025-08-05 22:05:42 +00:00
Member
Copy link

This pull request makes the naming changes requested in issues #109 and #110. It shouldn't affect behavior.

Notes

On the incoming branch, most methods implemented for engine::PartialMatrix use

let Self(entries) = self;

to destructure self. However, the implementation of IntoIterator for &'a PartialMatrix still uses the expression

let PartialMatrix(entries) = self;

because the Self version leads to a compiler error that says "the Self constructor can only be used with tuple or unit structs" (error code E0164). I think this is because Self is a reference type in this case. Here's a minimal working example you can play with at the Rust Playground.

struct Wrapper(i32);

trait Transform {
    fn transform(self) -> i32;
}

impl Transform for Wrapper {
    fn transform(self) -> i32 {
        let Self(inner) = self;
        10 * inner
    }
}

impl Transform for &Wrapper {
    fn transform(self) -> i32 {
        // using `Self` in place of `Wrapper` here leads to a compiler error.
        // dereferencing `self` doesn't help
        let Wrapper(inner) = self;
        100 * inner
    }
}

fn main() {
    let w = Wrapper(1);
    println!("Transformation result for each implementation");
    println!("  &Wrapper: {}", (&w).transform());
    println!("   Wrapper: {}", w.transform());
}
This pull request makes the naming changes requested in issues #109 and #110. It shouldn't affect behavior. ### Notes On the incoming branch, most methods implemented for `engine::PartialMatrix` use ```rust let Self(entries) = self; ``` to destructure `self`. However, the implementation of `IntoIterator` for `&'a PartialMatrix` still uses the expression ```rust let PartialMatrix(entries) = self; ``` because the `Self` version leads to a compiler error that says "the `Self` constructor can only be used with tuple or unit structs" (error code `E0164`). I think this is because `Self` is a reference type in this case. Here's a minimal working example you can play with at the [Rust Playground](https://play.rust-lang.org). ```rust struct Wrapper(i32); trait Transform { fn transform(self) -> i32; } impl Transform for Wrapper { fn transform(self) -> i32 { let Self(inner) = self; 10 * inner } } impl Transform for &Wrapper { fn transform(self) -> i32 { // using `Self` in place of `Wrapper` here leads to a compiler error. // dereferencing `self` doesn't help let Wrapper(inner) = self; 100 * inner } } fn main() { let w = Wrapper(1); println!("Transformation result for each implementation"); println!(" &Wrapper: {}", (&w).transform()); println!(" Wrapper: {}", w.transform()); } ```
Vectornaut added 2 commits 2025-08-05 22:05:42 +00:00 
In the process, make the loader names more consistent.
Use Self in implementations whenever possible
All checks were successful
/ test (pull_request) Successful in 3m34s
Details
1d03d1e8c2
glen reviewed 2025-08-06 15:54:11 +00:00
glen left a comment
Owner
Copy link

Just a small adjustment -- if we are going to improve the names of some of the loaders by using full words, we may as well do it consistently for all of them

Just a small adjustment -- if we are going to improve the names of some of the loaders by using full words, we may as well do it consistently for all of them
app-proto/src/components/test_assembly_chooser.rs Outdated
@ -247,3 +247,3 @@
// C-C "
// A-C -0.25 * φ^2 = -0.6545084971874737
fn load_tridim_icosahedron_assemb(assembly: &Assembly) {
fn load_tridim_icosahedron(assembly: &Assembly) {
glen commented 2025-08-06 15:36:01 +00:00
Owner
Copy link

given the changes above, should be either load_tridiminished_icosahedron or possibly load_tridiminished.

given the changes above, should be either `load_tridiminished_icosahedron` or possibly `load_tridiminished`.
Vectornaut commented 2025-08-06 21:15:50 +00:00
Author
Member
Copy link

Done in commit 83c1823!

Done in commit 83c1823!
glen marked this conversation as resolved
app-proto/src/components/test_assembly_chooser.rs Outdated
@ -410,3 +410,3 @@
// to finish describing the dodecahedral circle packing, set the inversive
// distance regulators to -1. some of the regulators have already been set
fn load_dodeca_packing_assemb(assembly: &Assembly) {
fn load_dodeca_packing(assembly: &Assembly) {
glen commented 2025-08-06 15:36:57 +00:00
Owner
Copy link

Similarly, should be either load_dodecahedral_packing or load_dodecahedral or possibly load_dodecahedron.

Similarly, should be either `load_dodecahedral_packing` or `load_dodecahedral` or possibly `load_dodecahedron`.
Vectornaut commented 2025-08-06 21:15:55 +00:00
Author
Member
Copy link

Done in commit 83c1823!

Done in commit 83c1823!
glen marked this conversation as resolved
Vectornaut added 1 commit 2025-08-06 21:15:03 +00:00
Expand abbreviations in test assembly names
All checks were successful
/ test (pull_request) Successful in 3m33s
Details
83c1823a4f 
This makes the naming more consistent, and having removed the `_assemb`
suffix gives us room to expand.
glen merged commit a4565281d5 into main 2025-08-07 23:24:09 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug

Something is not working

  
design

Issues that entail architecture/design decisions

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
maintenance

refactors andother behavior-orthogonal tasks

  
prospective

this becomes an issue once a particular feature is implemented or pull request is merged

  
question

More information is needed

  
regression

A bug that's resurfaced or a feature that's accidentally been removed

  
stub

We need to fill in the details of this issue

  
todo

Planned step in implementation

  
ui

issue concerning the user interface

  
wontfix

This won't be fixed

No labels
bug
design
duplicate
enhancement
maintenance
prospective
question
regression
stub
todo
ui
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: StudioInfinity/dyna3#111
No description provided.
Delete branch "Vectornaut/dyna3:simplify-names"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?