Skip to content

Critical bug in the DraftGizmos.GetGizmosPostfix() patch #142

Open

Description

@ZzZombo

It incorrectly expects the incoming sequence's item type to be Verse.Command when the original code works with its base class Verse.Gizmo. This breaks terribly when an item of such type is encountered in the sequence.

Additionally, it is inefficient in that 1) effectively causes double enumeration of the input sequence, 2) it constantly fetches the command icon instead of loading it once in the static constructor. I suggest you rewrite the function using the iterator protocol. Instead of copying the sequence into a list and then working on this copy make it yield every input item from the input sequence and when the condition is satisfied also yield the new gizmo.

It would look like this: Image

Addendum: it would also be good to avoid creating the new gizmo until the condition is satisfied, using the null conditional assignment so it is created at most once throughout the whole patch. Otherwise it causes unnecessary allocations to be garbage-collected later, in time-critial code.

Activity

  1. ZzZombo commented on Aug 9, 2025

    @ZzZombo
    Author

    I see that you had fixed the primary issue recently, but the code still has performance issues. I can't stress this nearly enough that is is executed in a time-critical game loop and it is imperative that it is as lightweight as possible. Avoiding pointless allocations would be a big help in this regard, not to mention the unneeded double enumeration of the sequence it entails.

  2. ZzZombo commented on Aug 22, 2025

    @ZzZombo
    Author

    @Taranchuk, @juanosarg, please take a look at the comment above.

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

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions