Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KTX pool's discard logic is inconsistent with the original Pool.discard(T) #476

Closed
metaphore opened this issue Jan 29, 2024 · 2 comments
Closed
Assignees
Labels
assets Issues of the ktx-assets module bug Reports of reproducible, confirmed unexpected behaviors
Milestone

Comments

@metaphore
Copy link

It seems like #363 brought a little inconsistency in how the pool discard logic works.

This is what the original discard method looks like:

protected void discard (T object) {
    reset(object);
}

And here's KTX:

override fun discard(element: Type) {
    discard(element)
}

So basically the reset call is gone and the default discard parameter to the KTX pool method is not replicating that logic.
I don't think it's possible to call the same reset instance method from the default discard lambda parameter.
So maybe adding an overload to the pool method without the discard parameter and without the discard(T) method override would be a good solution.

@czyzby czyzby added bug Reports of reproducible, confirmed unexpected behaviors assets Issues of the ktx-assets module labels Jan 30, 2024
@czyzby czyzby added this to the 1.12.1 milestone Jan 30, 2024
@czyzby czyzby self-assigned this Jan 30, 2024
@czyzby
Copy link
Member

czyzby commented Jan 30, 2024

You are right, adding the reset call would be problematic even if the lambda was given a Pool receiver, since it's a protected function. However, it's pretty easy to implement - all it does is calling reset on any object implementing the Poolable interface. I'll change it so that the default discard lambda attempts to reset the Poolable objects.

@czyzby
Copy link
Member

czyzby commented Jan 30, 2024

Should be available in the snapshot version shortly. Cheers.

@czyzby czyzby closed this as completed Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assets Issues of the ktx-assets module bug Reports of reproducible, confirmed unexpected behaviors
Projects
None yet
Development

No branches or pull requests

2 participants