Collision checking fixes

[mpragnay]

• Change collision checking to dynamic allocation to use all entities in vision range

• Remove corresponding constants(unnecessary, can be replaced with env->grid_map->grid_cell_count[idx] in all branches

[greptile-apps]

Greptile Summary

Replaced fixed-size static array allocation with dynamic allocation in collision checking to support variable entity counts per grid cell.

Key Changes:

• Removed MAX_ENTITIES_PER_CELL constant (was 30 entities)
• Modified checkNeighbors to return dynamically allocated array instead of filling caller-provided array
• Added free(entity_list) in compute_agent_metrics to prevent memory leak
• Removed Python binding for obsolete constant

Critical Issues Found:

• Missing NULL check after checkNeighbors call - will segfault if getGridIndex returns -1
• Missing NULL check after calloc - will segfault on allocation failure
• Unused variable min_neighbor_index calculated but never used

Confidence Score: 2/5

• Critical NULL pointer dereference bugs will cause segfaults in production
• Two critical logic bugs: missing NULL check after checkNeighbors (line 1272) and missing calloc failure check (line 1024). Both will cause segmentation faults when triggered. The dynamic allocation approach is good, but error handling is completely absent.
• pufferlib/ocean/drive/drive.h requires immediate attention to add NULL checks

Important Files Changed

Filename	Overview
pufferlib/ocean/drive/drive.h	Replaced fixed-size array with dynamic allocation in collision checking; missing NULL checks cause potential segfaults
pufferlib/ocean/env_binding.h	Removed Python binding for obsolete MAX_ENTITIES_PER_CELL constant

Sequence Diagram

sequenceDiagram
    participant CAM as compute_agent_metrics
    participant CN as checkNeighbors
    participant GI as getGridIndex
    participant GM as GridMap
    participant MEM as Memory

    CAM->>CN: checkNeighbors(env, x, y, offsets, 25, &list_size)
    CN->>GI: getGridIndex(env, x, y)
    alt Invalid position
        GI-->>CN: -1
        CN-->>CAM: NULL (no NULL check!)
        CAM->>CAM: Segfault accessing entity_list[i]
    else Valid position
        GI-->>CN: valid index
        CN->>CN: Calculate entity_list_count (loop through neighbors)
        CN->>MEM: calloc(entity_list_count, sizeof(GridMapEntity))
        alt Allocation fails
            MEM-->>CN: NULL (no check!)
            CN->>GM: memcpy to NULL pointer
            CN->>CAM: Segfault
        else Allocation succeeds
            MEM-->>CN: entity_list pointer
            CN->>GM: memcpy entities from cells[neighborIndex]
            CN-->>CAM: entity_list pointer
            CAM->>CAM: Process entities in loop
            CAM->>CAM: Check collisions with entities
            CAM->>MEM: free(entity_list)
        end
    end

Loading

[greptile-apps]

_{2 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

style: min_neighbor_index is calculated but never used. Remove unused variable.

Suggested change

	int min_neighbor_index = INT16_MAX;
	for (int i = 0; i < offset_size; i++) {
	int nx = gridX + local_offsets[i][0];
	int ny = gridY + local_offsets[i][1];
	if (nx < 0 || nx >= env->grid_map->grid_cols || ny < 0 || ny >= env->grid_map->grid_rows)
	continue;
	int neighborIndex = ny * env->grid_map->grid_cols + nx;
	min_neighbor_index = fmin(min_neighbor_index, neighborIndex);
	// Calculate entities count in neighboring cells
	for (int i = 0; i < offset_size; i++) {

Prompt To Fix With AI

This is a comment left during a code review.
Path: pufferlib/ocean/drive/drive.h
Line: 1009:1016

Comment:
**style:** `min_neighbor_index` is calculated but never used. Remove unused variable.

```suggestion
    // Calculate entities count in neighboring cells
    for (int i = 0; i < offset_size; i++) {
```

How can I resolve this? If you propose a fix, please make it concise.

[nadarenator]

How much does this impact SPS?

[mpragnay]

How much does this impact SPS?

How much does this impact SPS?

No real difference observed.

[mpragnay]

How much does this impact SPS?

How much does this impact SPS?

No real difference observed.

Pink run is in main br