Skip to content

feat: add shape name to obj loader shapes#433

Merged
Divengerss merged 2 commits into
mainfrom
objloader-shape-name
Jan 24, 2026
Merged

feat: add shape name to obj loader shapes#433
Divengerss merged 2 commits into
mainfrom
objloader-shape-name

Conversation

@ripel2

@ripel2 ripel2 commented Jan 24, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Not related to any issues.

Small addition to the OBJLoader to add the name of the shapes from .obj files to ESQ's Shape struct

Summary by CodeRabbit

  • New Features
    • Better preservation of shape metadata so imported 3D objects retain their names.
    • Materials now capture ambient texture identifiers during import, improving texture handling and display.

✏️ Tip: You can customize this high-level summary in your review settings.

@ripel2 ripel2 requested a review from a team January 24, 2026 13:26
@coderabbitai

coderabbitai Bot commented Jan 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Added a public name field and getter to Object::Resource::Shape; populated that field in the OBJ loader. Also added ambientTexName to Component::Material and updated tests to destructure the extra shape tuple element.

Changes

Cohort / File(s) Summary
Shape resource definition
src/plugin/object/src/resource/Shape.hpp		 Added public std::string name member and const std::string &GetName() const accessor to Object::Resource::Shape.
OBJ loader implementation
src/plugin/object/src/resource/OBJLoader.cpp		 Populates shapeResource.name from source shape data; sets Component::Material::ambientTexName from mat.ambient_texname.
Tests updated
src/plugin/object/tests/OBJLoaderTest.cpp		 Updated test loops to destructure three elements (mesh, material, name) from GetShapes() results; assertions unchanged otherwise.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I nibble bytes and name each shape with glee,
Ambient threads tucked in the material tree,
A hop, a tweak, the loader hums anew—
Small changes stitched, the render peeks through. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: add shape name to obj loader shapes' accurately and clearly summarizes the main change in the pull request—adding a shape name field to the OBJLoader implementation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ripel2 ripel2 self-assigned this Jan 24, 2026
MasterLaplace
MasterLaplace previously approved these changes Jan 24, 2026
View reviewed changes
@MasterLaplace MasterLaplace requested review from a team and MasterLaplace and removed request for MasterLaplace January 24, 2026 13:30
@sonarqubecloud

sonarqubecloud Bot commented Jan 24, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@Divengerss Divengerss merged commit a070a8f into main Jan 24, 2026
16 checks passed
@Divengerss Divengerss deleted the objloader-shape-name branch January 24, 2026 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@Divengerss Divengerss Divengerss approved these changes

@MasterLaplace MasterLaplace MasterLaplace left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@ripel2 ripel2

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ripel2 @MasterLaplace @Divengerss