Skip to content

Fix bundle freeing: Bitmap resources not being freed#153

Open
rory-cd wants to merge 1 commit into
thoth-tech:mainfrom
rory-cd:fix/free-resource-bundle-bitmaps
Open

Fix bundle freeing: Bitmap resources not being freed#153
rory-cd wants to merge 1 commit into
thoth-tech:mainfrom
rory-cd:fix/free-resource-bundle-bitmaps

Conversation

@rory-cd
Copy link
Copy Markdown

@rory-cd rory-cd commented Jan 19, 2026

Description

Background

SplashKit bundles allow sets of resources to be loaded and freed as a group. For more information, see the documentation. The free_resource_bundle function is supposed to free every resource in the bundle at once.

Problem

The free_resource_bundle function currently does not free bitmaps. This was shown in the "Bundles" test results - every other resource was freed successfully, except the bitmaps.

Cause

When a bundle is loaded, each line of the bundle text file is analysed and run through a SWITCH in the load_resource_bundle function, loading the resource based on its type. Then the resources are added to a list (vector), which is stored as a field in the bundle. Meaning each bundle in memory has a list of corresponding resources. However, after the resources are loaded, their existence is checked to ensure the loading was successful. If it wasn't, the function returns early and the resource isn't added to the list.

In the case of a bitmap, we have:

case IMAGE_RESOURCE:
    rb_load_bitmap(line_name, line_path);
    if ( ! has_resource_bundle(line_name) ) return;
    break;

Which means the condition checks for the bundle, not a bitmap. So the bitmap is loaded, but not added to the bundle's list of resources, and therefore not freed by free_resource_bundle.

Fix

This is a very small and simple fix, changing the IMAGE_RESOURCE case so it checks has_bitmap rather than has_resource_bundle.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Ran the "Bundles" test in sktest in WSL and MSYS2. Also ran skunit_tests.

Results of Bundles test

bundle-test-results

Testing Checklist

  • Tested with sktest
  • Tested with skunit_tests

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have requested a review from ... on the Pull Request

Copy link
Copy Markdown

@222448082Ashen 222448082Ashen left a comment

Choose a reason for hiding this comment

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

General Information

  • Type of Change: Bug fix

Code Quality

  • Repository: Correct. The PR is made to splashkit-core.
  • Readability: High. The fix is a straightforward correction of a logic error.
  • Maintainability: High. Aligning the check with the resource type (IMAGE_RESOURCE -> has_bitmap) ensures consistency with other resource types in the same switch block.

Functionality

  • Correctness: The fix is correct. In load_resource_bundle, the code for IMAGE_RESOURCE was previously checking has_resource_bundle(line_name) after attempting to load a bitmap. Since a bitmap is not a bundle, this check would likely fail, causing the function to return early and fail to add the bitmap to the bundle's resource list.
    case IMAGE_RESOURCE:
        rb_load_bitmap(line_name, line_path);
        if ( ! has_bitmap(line_name) ) return; // Corrected from has_resource_bundle
        break;
  • Impact on Existing Functionality: Resolves a bug that prevented bitmaps from being correctly tracked and managed within resource bundles.

Testing

  • Test Results: Logic check confirms that the IMAGE_RESOURCE case now correctly mirrors the logic of FONT_RESOURCE, SOUND_RESOURCE, etc., which all check for their respective resource types.

Documentation

  • Documentation: N/A.

Pull Request Details

  • PR Description: Correctly identifies the logic error in bundles.cpp.
  • Checklist Completion: All relevant items reviewed.

@222448082Ashen
Copy link
Copy Markdown

Recommendations & Observations

  1. Consistency: This fix ensures that all resource types in the load_resource_bundle function are handled consistently. It's a textbook example of fixing a copy-paste error that could have led to subtle resource management bugs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants