GraphiteEditor/Graphite

Find and replace all magic number node parameter indexes with ::INDEX const

Open

#1,490 opened on Dec 7, 2023

View on GitHub
 (8 comments) (0 reactions) (0 assignees)Rust (25,662 stars) (1,176 forks)batch import
Good First Issue

Description

This issue must be completed WITHOUT ANY AI USAGE. It does not involve writing code, just searching through lots of code diligently and without missing important details. This is something AI is not able to be trusted for.

The task here is to use various searching and go-to-references/implementations techniques to find as many magic number indexes as possible for node parameters, and replace them with their node parameter's ::INDEX constant.

For example, two parameters of the Stroke node:

  • graphene_std::vector::stroke::ColorInput::INDEX (equals 1)
  • graphene_std::vector::stroke::DashLengthsInput::<Vec<f64>>::INDEX (equals 8)

Since this ::INDEX system is newer, there are many places we have magic numbers for indexes like 1 and 8 lying around in the code. They are probably often used to generate values for specific function calls. For example, InputConnector::node is one such function that can have its usages searched to find plenty of examples where magic number indexes are passed in, that should be replaced by an ::INDEX const instead. In some cases, this could be indirect, like if that function is passed a variable set elsewhere. And there are likely other functions besides that one, too, that are receiving magic number indexes.

After those techniques are completed, a final check should be done by manually checking through the ~1.5k (perhaps less after the other replacements are completed) results from this project-wide search:

  • Enable regex but not word boundaries or capitalization
  • Query: (?<!\d|[a-z]|\.|\\|'|"|= ?|> |return |[-+/*%] ?|_|; |NodeId\(|box\[|bounds\[|points\[|points_viewport\[|locations\[|display_decimal_places\(|_vertical\[|_horizontal\[|segments\[|handles\[|corners\[|radii\[|default\(|\.skip\(|\.min\(|\.max\(|digit: |Step |Case |Test |", |Vec2::new\(|powi\(|\[\d\]\[|\.0\[|[\s(][a-z]\[)[1-3]?\d(?!\d|[a-z]|\.|'|"|_|;.| [<>=]| ?px|\]\[|\]\.)
  • Files to include: editor/**/*.rs, node-graph/**/*.rs
  • Files to exclude: document_migration.rs, document_node_definitions.rs

Contributor guide

Tech stack
rust
Domain
backenddeveloper experience
Issue type
refactor
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
3
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
3-5 days
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
stale
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
clear
Prerequisites
Rustcode searching skillsunderstanding of node graph structureattention to detail
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
40
Research direction
Search through the `editor` and `node graph` Rust source directories for magic number indexes used for node parameters. Use the provided regex `(?<!\d|[a z]|\.|\\|'|"|= ?|> |return |[ +/*%] ?| |; |NodeId\(|box\[|bounds\[|points\[|points viewport\[|locations\[|display decimal places\(| vertical\[| horizontal\[|segments\[|handles\[|corners\[|radii\[|default\(|\.skip\(|\.min\(|\.max\(|digit: |Step |Case |Test |", |Vec2::new\(|powi\(|\[\d\]\[|\.0\[|[\s(][a z]\[)[1-3]?\d(?!\d|[a z]|\.|'|"| |;.| [<>=]| ?px|\]\[|\]\.)` to filter potential candidates, excluding files `document migration.rs` and `document node definitions.rs`. Replace each magic number with the corresponding `::INDEX` constant (e.g., `graphene std::vector::stroke::ColorInput::INDEX`). Refer to the example in the issue and check the `InputConnector::node` function in `editor/src/messages/portfolio/document/utility types/network interface.rs`. Manually verify all ~1.5k regex matches after replacements.