fix: guard MapMeta#getMapView against IllegalStateException#154
fix: guard MapMeta#getMapView against IllegalStateException#154HyacinthHaru wants to merge 1 commit intoLOOHP:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Guards map metadata access against a Paper/Leaves edge case where MapMeta#getMapView() can throw IllegalStateException even after hasMapView() checks, preventing event/task spam and improving resilience to corrupted map meta.
Changes:
- Hardened
MapUtils#getItemMapViewwithtry/catch (IllegalStateException)and reused it fromrayTraceTargetImageMap. - Added safe
MapViewretrieval and additional null/bounds guards inImageFilledMapUtilsto handle stale/corrupt map info. - Replaced direct
MapMeta#getMapView()usage inMapMarkerEditManagerwithMapUtils#getItemMapViewto avoid unsafe calls.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| common/src/main/java/com/loohp/imageframe/utils/MapUtils.java | Adds exception-safe MapView extraction and reuses it in raytracing. |
| common/src/main/java/com/loohp/imageframe/utils/ImageFilledMapUtils.java | Adds safe MapView retrieval + defensive guards for invalid/stale filled-map info. |
| common/src/main/java/com/loohp/imageframe/objectholders/MapMarkerEditManager.java | Avoids unsafe direct map meta access by using MapUtils#getItemMapView. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ItemStack itemStack = itemFrame.getItem(); | ||
| if (itemStack == null || itemStack.getType().equals(Material.AIR)) { | ||
| continue; | ||
| } | ||
| if (!itemStack.hasItemMeta()) { | ||
| continue; | ||
| } | ||
| ItemMeta itemMeta = itemStack.getItemMeta(); | ||
| if (!(itemMeta instanceof MapMeta)) { | ||
| continue; | ||
| } | ||
| MapMeta mapMeta = (MapMeta) itemMeta; | ||
| if (!mapMeta.hasMapView()) { | ||
| continue; | ||
| } | ||
| MapView mapView = mapMeta.getMapView(); | ||
| MapView mapView = MapUtils.getItemMapView(itemStack); | ||
| if (mapView == null) { | ||
| continue; |
There was a problem hiding this comment.
MapUtils.getItemMapView() performs tryDeleteBlankDataFile(...) (filesystem existence/length checks) when Settings.TryDeleteBlankMapFiles is enabled. Calling it from this per-tick edit task can introduce repeated IO and potential lag under that setting. Consider adding/using a variant that only does the safe MapView extraction (no blank-file cleanup) for hot paths like marker editing/raytracing.
| private static MapView getMapViewSafely(MapMeta mapMeta) { | ||
| try { | ||
| if (!mapMeta.hasMapView()) { | ||
| return null; | ||
| } | ||
| return mapMeta.getMapView(); | ||
| } catch (IllegalStateException ignored) { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
getMapViewSafely(MapMeta) duplicates the same hasMapView() + getMapView() try/catch logic now present in MapUtils.getItemMapView(...). To avoid the two implementations drifting over time, consider centralizing this into a shared helper (e.g., a MapUtils.getMapViewSafely(MapMeta) or an overload of getItemMapView that accepts MapMeta / skips cleanup).
1 participant
On Leaves/Paper 1.21.10,
MapMeta#getMapView()can throw -IllegalStateException: Item does not have map associated - check hasMapView() first!even after
hasMapView()checks in some cases (invalid/corrupted map meta state).This caused event handler errors in:
EntityPickupItemEventInventoryClickEventSo, I changed
MapViewretrieval withtry/catch IllegalStateExceptionin:ImageFilledMapUtilsMapUtils#getItemMapViewImageFilledMapUtilsfor stale map info.MapUtils#getItemMapViewinMapMarkerEditManagerto avoid direct unsafe calls.plugin now skips malformed map metadata gracefully instead of throwing and spamming console errors.