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.

Sign up for GitHub

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

Jump to bottom

Adds all missing lineart images in gun examine #6905

Merged
merged 3 commits into from
Aug 22, 2024
Merged

Adds all missing lineart images in gun examine #6905

merged 3 commits into from
Aug 22, 2024

Conversation

Blundir
Copy link
Contributor

@Blundir Blundir commented Aug 9, 2024 •
edited
Loading

About the pull request

Adds all missing ones mentioned in #5733, i'll handle some others and resprites of old ones in another pr.

ljaipkGMkF

All sprites by me.

Explain why it's good for the game

Missing icons bad

Testing Photographs and Procedure

Screenshots & Videos

dreammaker_Xvmf2lXvQS

Changelog

🆑
imageadd: adds lineart images for almost all guns that didn't have it
code: missing lineart error is now a lint
/:cl:

@github-actions github-actions bot added Sprites Remove the soul from the game. Mapping did you remember to save in tgm format? labels Aug 9, 2024
@Blundir Blundir removed the Mapping did you remember to save in tgm format? label Aug 9, 2024
@Blundir Blundir linked an issue Aug 9, 2024 that may be closed by this pull request
Various guns have no lineart sprite when viewing weapon stats #5733
Closed
42 tasks
@Drulikar
Copy link
Contributor

Drulikar commented Aug 9, 2024

Of note the weapons that remain w/o lineart are:

[13:21:22]DEBUG: /obj/item/weapon/gun/lever_action does not have a valid lineart icon state, icon=icons/obj/items/weapons/guns/lineart.dmi, icon_state="r4t-placeholder"

[13:21:22]DEBUG: /obj/item/weapon/gun/lever_action/r4t does not have a valid lineart icon state, icon=icons/obj/items/weapons/guns/lineart.dmi, icon_state="r4t"

[13:21:22]DEBUG: /obj/item/weapon/gun/revolver/m44/custom/pkd_special/k2049 does not have a valid lineart icon state, icon=icons/obj/items/weapons/guns/lineart.dmi, icon_state="lapd_2049"

[13:21:22]DEBUG: /obj/item/weapon/gun/shotgun/double/twobore does not have a valid lineart icon state, icon=icons/obj/items/weapons/guns/lineart.dmi, icon_state="twobore"

[13:21:22]DEBUG: /obj/item/weapon/gun/smg/uzi does not have a valid lineart icon state, icon=icons/obj/items/weapons/guns/lineart.dmi, icon_state="uzi"

@Blundir Blundir changed the title Adds almost all missing lineart images in gun examine Adds all missing lineart images in gun examine Aug 10, 2024
Drulikar
Drulikar reviewed Aug 10, 2024
View reviewed changes
Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

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

No more debug logs show up for missing lineart now. The debug could be escalated to an error if we wanted to enforce all weapons have lineart now:

cmss13/code/modules/asset_cache/asset_list_items.dm

Lines 393 to 394 in 04b3134

// upgrade this to a stack_trace once all guns have a lineart and we want to lint against that
log_debug("[current_gun] does not have a valid lineart icon state, icon=[icon_file], icon_state=[json_encode(icon_state)]")

@Blundir
Copy link
Contributor Author

Blundir commented Aug 10, 2024

No more debug logs show up for missing lineart now. The debug could be escalated to an error if we wanted to enforce all weapons have lineart now:

cmss13/code/modules/asset_cache/asset_list_items.dm

Lines 393 to 394 in 04b3134

// upgrade this to a stack_trace once all guns have a lineart and we want to lint against that
log_debug("[current_gun] does not have a valid lineart icon state, icon=[icon_file], icon_state=[json_encode(icon_state)]")

That'd be ideal, they are not hard to make, also would be easier to maintain it in newer prs with such reminder

@Drulikar
Copy link
Contributor

No more debug logs show up for missing lineart now. The debug could be escalated to an error if we wanted to enforce all weapons have lineart now:

cmss13/code/modules/asset_cache/asset_list_items.dm

Lines 393 to 394 in 04b3134

// upgrade this to a stack_trace once all guns have a lineart and we want to lint against that
log_debug("[current_gun] does not have a valid lineart icon state, icon=[icon_file], icon_state=[json_encode(icon_state)]")

That'd be ideal, they are not hard to make, also would be easier to maintain it in newer prs with such reminder

Then feel free to do what the comment says in the above linked code

@github-actions github-actions bot added the Code Improvement Make the code longer label Aug 12, 2024
Drulikar
Drulikar approved these changes Aug 13, 2024
View reviewed changes
Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

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

code is fine

@harryob harryob added the Sprites Approved confirmed no stray pixels label Aug 22, 2024
@harryob harryob added this pull request to the merge queue Aug 22, 2024
github-merge-queue bot pushed a commit that referenced this pull request Aug 22, 2024
@Blundir
Adds all missing lineart images in gun examine (#6905)
fecfc71 
<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may
not be viewable. -->

# About the pull request

Adds all missing ones mentioned in
#5733, i'll handle some
others and resprites of old ones in another pr.


![ljaipkGMkF](https://github.com/user-attachments/assets/769d8beb-98e0-4689-852b-96c7f4e9e7ef)


All sprites by me.


# Explain why it's good for the game

Missing icons bad

# Testing Photographs and Procedure
<!-- Include any screenshots/videos/debugging steps of the modified code
functioning successfully, ideally including edge cases. -->


<details>
<summary>Screenshots & Videos</summary>


![dreammaker_Xvmf2lXvQS](https://github.com/user-attachments/assets/d63275bd-3012-4dc9-909e-541ec11715c8)


</details>


# Changelog

<!-- If your PR modifies aspects of the game that can be concretely
observed by players or admins you should add a changelog. If your change
does NOT meet this description, remove this section. Be sure to properly
label your changes in the changelog. Please note that maintainers freely
reserve the right to remove and add tags should they deem it
appropriate. You can attempt to finagle the system all you want, but
it's best to shoot for clear communication right off the bat. -->
<!-- If you add a name after the ':cl', that name will be used in the
changelog. You must add your CKEY after the CL if your GitHub name
doesn't match. Maintainers freely reserve the right to remove and add
tags should they deem it appropriate. -->

:cl:
imageadd: adds lineart images for almost all guns that didn't have it
code: missing lineart error is now a lint
/:cl:

<!-- Both :cl:'s are required for the changelog to work! -->
Merged via the queue into cmss13-devs:master with commit 027781c Aug 22, 2024
29 checks passed
cm13-github added a commit that referenced this pull request Aug 22, 2024
@cm13-github
Automatic changelog for PR #6905 [ci skip]
818d811
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Drulikar Drulikar Drulikar approved these changes

Assignees
No one assigned
Labels
Code Improvement Make the code longer Sprites Approved confirmed no stray pixels Sprites Remove the soul from the game.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Various guns have no lineart sprite when viewing weapon stats
3 participants
@Blundir @Drulikar @harryob