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

(#485) make drawable and drawableName optional for scene2d image factory #487

Merged
merged 2 commits into from
Jul 30, 2024

Conversation

Quillraven
Copy link
Contributor

PR for #485.

I did not adjust the factory methods for Texture, TextureRegion and NinePatch because I think they will throw an exception, but more importantly, I think in those cases it does not make sense. Drawable and drawable name do make sense because in a Skin, from what I know, you can reference Drawables and in the case of an image they are optional.

scene2d/src/main/kotlin/ktx/scene2d/factory.kt Outdated Show resolved Hide resolved
scene2d/src/test/kotlin/ktx/scene2d/factoryTest.kt Outdated Show resolved Hide resolved
scene2d/src/test/kotlin/ktx/scene2d/factoryTest.kt Outdated Show resolved Hide resolved
@czyzby
Copy link
Member

czyzby commented Jul 29, 2024

Thanks for the PR, I left some minor comments.

- revert drawableName change
- fix asterisk import
@Quillraven
Copy link
Contributor Author

Quillraven commented Jul 30, 2024

Issues should be resolved now. Regarding .editorconfig: I had a quick look and I could only find that there are IntelliJ specific properties that are supported. I guess the "star_import" properties are the ones that would be needed. But since they have the prefix "ij_" it will most likely only work in IntelliJ.

image

The thing that I did to solve it was a setting directly in IntelliJ:
image

The default setting was "Use import with '*' when at least 5 names used / 3 names used

@Quillraven Quillraven requested a review from czyzby July 30, 2024 10:59
@czyzby czyzby merged commit ac8e6e9 into libktx:develop Jul 30, 2024
3 checks passed
@czyzby
Copy link
Member

czyzby commented Jul 30, 2024

Thanks, the changes should be available in the snapshot release shortly. I'll try to update the changelog and possibly push a new release over the weekend.

@czyzby
Copy link
Member

czyzby commented Jul 30, 2024

Just wanted to say that automatic snapshot upload failed, since I had to regenerate an access token into Maven central repository. Should be fixed now.

@Quillraven
Copy link
Contributor Author

Quillraven commented Jul 30, 2024

Haha had the same issue with Fleks the last time. Welcome to the club 😉

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