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

Tweaks: Many module positioning fixes and new tweaks #30

Merged
merged 3 commits into from
Aug 24, 2021
Merged

Tweaks: Many module positioning fixes and new tweaks #30

merged 3 commits into from
Aug 24, 2021

Conversation

eXish
Copy link
Contributor

@eXish eXish commented Aug 16, 2021

List of corrections:

  • Colour Flash and its translated variants were slightly too far to the left on the x-axis
  • Combination Lock's status light was floating
  • The Matrix, Memorable Buttons, Vexillology and Strike/Solve were slightly too far up on the y-axis
  • Vexillology had the module's selectable as Pass Through
  • The light scale fixer only scaled down the first light it came in contact with and ignored the rest
  • The Jack-O’-Lantern and The Sphere did not scale their lights properly
  • Needy Math's question and answer displays were floating
  • Needy Math, Filibuster, Needy Mrs. Bob, Draw, Rapid Buttons, Simon Squawks, and Triangle Buttons had z-fighting in their timer displays
  • Draw had endless playing sound, visual glitches, and being able to play the mod when it was deactivated
  • Grocery Store allowed for buttons to be pressed after it was solved

In many cases where I had to move a module around other items that were already correct got moved so they would have to get moved back, this is why you see some mods being added to the fix highlight section when they were fine before.

List of corrections:
- Colour Flash and its translated variants were slightly too far to the left on the x-axis
- Combination Lock's status light was floating
- The Matrix, Memorable Buttons, Vexillology and Strike/Solve were slightly too far up on the y-axis
- Vexillology had the module's selectable as Pass Through
- The light scale fixer only scaled down the first light it came in contact with and ignored the rest
- The Jack-O’-Lantern and The Sphere did not scale their lights properly
- Needy Math's question and answer displays were floating
- Needy Math, Filibuster, Needy Mrs. Bob, Draw, Rapid Buttons, Simon Squawks, and Triangle Buttons had z-fighting in their timer displays
In many cases where I had to move a module around other items that were already correct got moved so they would have to get moved back, this is why you see some mods being added to the fix highlight section when they were fine before.
Copy link
Owner

@samfundev samfundev left a comment

Choose a reason for hiding this comment

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

This code should really be refactored. In the meantime, some improvements can be made.

Comment on lines 302 to 312
string[] lightSuffixes = { "Top", "1", "2", "3", "4" };
for (int i = 0; i < 5; i++)
component.transform.Find("hinge/wheelHolder/outerWheel/centre/centralOrb/yellowLights/yellowLight" + lightSuffixes[i]).GetComponent<Light>().range *= component.transform.lossyScale.x;
for (int i = 0; i < 5; i++)
component.transform.Find("hinge/wheelHolder/outerWheel/centre/centralOrb/redLights/redLight" + lightSuffixes[i]).GetComponent<Light>().range *= component.transform.lossyScale.x;
for (int i = 0; i < 5; i++)
component.transform.Find("hinge/wheelHolder/outerWheel/centre/centralOrb/greenLights/greenLight" + lightSuffixes[i]).GetComponent<Light>().range *= component.transform.lossyScale.x;
for (int i = 0; i < 5; i++)
component.transform.Find("hinge/wheelHolder/outerWheel/centre/centralOrb/blueLights/blueLight" + lightSuffixes[i]).GetComponent<Light>().range *= component.transform.lossyScale.x;
for (int i = 0; i < 5; i++)
component.transform.Find("hinge/wheelHolder/outerWheel/centre/centralOrb/pinkLights/pinkLight" + lightSuffixes[i]).GetComponent<Light>().range *= component.transform.lossyScale.x;
Copy link
Owner

Choose a reason for hiding this comment

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

This can be simplified to:

Suggested change
string[] lightSuffixes = { "Top", "1", "2", "3", "4" };
for (int i = 0; i < 5; i++)
component.transform.Find("hinge/wheelHolder/outerWheel/centre/centralOrb/yellowLights/yellowLight" + lightSuffixes[i]).GetComponent<Light>().range *= component.transform.lossyScale.x;
for (int i = 0; i < 5; i++)
component.transform.Find("hinge/wheelHolder/outerWheel/centre/centralOrb/redLights/redLight" + lightSuffixes[i]).GetComponent<Light>().range *= component.transform.lossyScale.x;
for (int i = 0; i < 5; i++)
component.transform.Find("hinge/wheelHolder/outerWheel/centre/centralOrb/greenLights/greenLight" + lightSuffixes[i]).GetComponent<Light>().range *= component.transform.lossyScale.x;
for (int i = 0; i < 5; i++)
component.transform.Find("hinge/wheelHolder/outerWheel/centre/centralOrb/blueLights/blueLight" + lightSuffixes[i]).GetComponent<Light>().range *= component.transform.lossyScale.x;
for (int i = 0; i < 5; i++)
component.transform.Find("hinge/wheelHolder/outerWheel/centre/centralOrb/pinkLights/pinkLight" + lightSuffixes[i]).GetComponent<Light>().range *= component.transform.lossyScale.x;
foreach (var color in new[] { "yellow", "red", "green", "blue", "pink" })
{
foreach (var suffix in new[] { "Top", "1", "2", "3", "4" })
{
component.transform.Find($"hinge/wheelHolder/outerWheel/centre/centralOrb/{color}Lights/{color}Light{suffix}").GetComponent<Light>().range *= component.transform.lossyScale.x;
}
}

case "ColourFlashES":
case "ColourFlashPL":
// This fixes the position of the module itself (but keeps the status light in its original location)
component.transform.Find("ModuleBackground").transform.localPosition = Vector3.zero;
Copy link
Owner

Choose a reason for hiding this comment

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

I didn't realize this before but Find() returns a transform, so that .transform is redundant.

Suggested change
component.transform.Find("ModuleBackground").transform.localPosition = Vector3.zero;
component.transform.Find("ModuleBackground").localPosition = Vector3.zero;

Comment on lines 267 to 268
for (int i = 0; i < component.transform.childCount; i++)
component.transform.GetChild(i).transform.localPosition -= new Vector3(0, 0.0053061005f, 0);
Copy link
Owner

Choose a reason for hiding this comment

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

There's a better way to iterate over children:

Suggested change
for (int i = 0; i < component.transform.childCount; i++)
component.transform.GetChild(i).transform.localPosition -= new Vector3(0, 0.0053061005f, 0);
foreach (Transform child in component.transform)
child.localPosition -= new Vector3(0, 0.0053061005f, 0);

@eXish eXish changed the title Tweaks: Many module positioning fixes Tweaks: Many module positioning fixes and new tweaks Aug 23, 2021
@samfundev
Copy link
Owner

Thanks for the PR, merging it in!

@samfundev samfundev merged commit abe506c into samfundev:master Aug 24, 2021
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