Skip to content

Conversation

@edenhaus
Copy link
Member

No description provided.

@edenhaus edenhaus added the pr: refactor PR with code refactoring label Jun 19, 2025
@codecov
Copy link

codecov bot commented Jun 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.74%. Comparing base (d82619b) to head (c0dda4d).
Report is 4 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1033      +/-   ##
==========================================
- Coverage   93.77%   93.74%   -0.03%     
==========================================
  Files         129      129              
  Lines        5012     4988      -24     
  Branches      327      327              
==========================================
- Hits         4700     4676      -24     
  Misses        249      249              
  Partials       63       63              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed-hq
Copy link

codspeed-hq bot commented Jun 19, 2025

CodSpeed Performance Report

Merging #1033 will not alter performance

Comparing optimize-map (c0dda4d) with dev (c40b1e8)

Summary

✅ 6 untouched benchmarks

@edenhaus edenhaus marked this pull request as ready for review June 20, 2025 13:14
Copilot AI review requested due to automatic review settings June 20, 2025 13:14
@edenhaus edenhaus merged commit afd5b29 into dev Jun 20, 2025
27 checks passed
@edenhaus edenhaus deleted the optimize-map branch June 20, 2025 13:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR streamlines map rendering by replacing dynamic palettes with compile-time constants, refactors data parsing loops for performance, and optimizes change detection in map pieces.

  • Replaced Lazy<Vec<u8>> palettes with const slices and fixed-size transparency arrays
  • Switched byte-order reads to from_le_bytes and simplified iteration closures to for loops
  • Updated update_points to early-exit on unchanged data and adjusted tests for slice inputs

// Newer bots will return a different pixel index per room
// mapping all to the floor color
let pixel = if *pixel_idx > *MAP_IMAGE_PALETTE_LEN as u8 {
let pixel = if pixel_idx > MAP_IMAGE_PALETTE_LEN {
Copy link

Copilot AI Jun 20, 2025

Choose a reason for hiding this comment

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

The check pixel_idx > MAP_IMAGE_PALETTE_LEN allows pixel_idx == MAP_IMAGE_PALETTE_LEN to slip through, leading to an out-of-bounds palette index. Change the condition to >= to correctly treat indices equal to the palette length as floor.

Suggested change
let pixel = if pixel_idx > MAP_IMAGE_PALETTE_LEN {
let pixel = if pixel_idx >= MAP_IMAGE_PALETTE_LEN {

Copilot uses AI. Check for mistakes.
}
});

let mut points = Vec::with_capacity(subset.coordinates.len() / 2);
Copy link

Copilot AI Jun 20, 2025

Choose a reason for hiding this comment

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

[nitpick] Pre-allocating based on the string length may over-allocate. Consider using numbers.size_hint() or collecting into a temporary Vec<f32> first to get an accurate capacity estimate.

Suggested change
let mut points = Vec::with_capacity(subset.coordinates.len() / 2);
let mut points = Vec::with_capacity(numbers.size_hint().0 / 2);

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: refactor PR with code refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants