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

scene2d image without drawable #485

Closed
Quillraven opened this issue Jun 22, 2024 · 3 comments
Closed

scene2d image without drawable #485

Quillraven opened this issue Jun 22, 2024 · 3 comments
Assignees
Labels
scene2d Issues of the ktx-scene2d module and Scene2D integration in general
Milestone

Comments

@Quillraven
Copy link
Contributor

Hi,

right now the image scene2d extension requires a non-null Drawable:

@Scene2dDsl
@OptIn(ExperimentalContracts::class)
inline fun <S> KWidget<S>.image(
  drawable: Drawable,
  init: (@Scene2dDsl Image).(S) -> Unit = {},
): Image {
  contract { callsInPlace(init, InvocationKind.EXACTLY_ONCE) }
  return actor(Image(drawable), init)
}

However, it is valid to pass "null" as a drawable and this can be useful if the real drawable is set later on but you still want to add the image to the table to have the correct layout.

Would it be okay to make it Drawable? instead of Drawable? This might be also true for other extensions with drawables but I did not check them all.

Or is there a different solution to it?

@czyzby czyzby added the scene2d Issues of the ktx-scene2d module and Scene2D integration in general label Jun 22, 2024
@czyzby
Copy link
Member

czyzby commented Jun 22, 2024

If an image without a drawable can exist and might make sense for some use cases, it should be possible to create with the DSL. This might be an oversight, but it can be easily amended without breaking the API. Drawable? with a null default could be a way to solve this, I just have to check first if an image without a drawable isn't otherwise broken (e.g., thowing NPE on draw). Thanks for bringing this up.

@Quillraven
Copy link
Contributor Author

Imo this should be just fine and at least in my case it works. Here is also the official documentation of setDrawable which mentions it may be null:

image

Quillraven pushed a commit to Quillraven/ktx that referenced this issue Jul 29, 2024
Quillraven pushed a commit to Quillraven/ktx that referenced this issue Jul 30, 2024
- revert drawableName change
- fix asterisk import
czyzby pushed a commit that referenced this issue Jul 30, 2024
* (#485) make drawableName optional for scene2d image factory

* (#485) make drawable optional for scene2d image factory

- revert drawableName change
- fix asterisk import

---------

Co-authored-by: Simon Klausner <[email protected]>
@Quillraven
Copy link
Contributor Author

Will be fixed by the linked PR above

czyzby added a commit that referenced this issue Aug 25, 2024
@czyzby czyzby added this to the 1.12.1 milestone Aug 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scene2d Issues of the ktx-scene2d module and Scene2D integration in general
Projects
None yet
Development

No branches or pull requests

2 participants