Skip to content

Simplify antenna on neobread_bot#5

Open
o76923 wants to merge 2 commits into
olivvybee:mainfrom
o76923:neobread_bot_fix
Open

Simplify antenna on neobread_bot#5
o76923 wants to merge 2 commits into
olivvybee:mainfrom
o76923:neobread_bot_fix

Conversation

@o76923
Copy link
Copy Markdown

@o76923 o76923 commented Sep 13, 2024

The path on the outline was so complicated that it was driving me nuts. I replaced it with a greatly simplified version.

The x and y on the eye filter should be reduced to -2 since the width and height are 5 now.

2 to the left of the object + the object + 2 to the right of the object = 5 total object widths.
Comment thread neobread/neobread_bot.svg
<svg width="100%" height="100%" viewBox="0 0 256 256" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" xml:space="preserve" xmlns:serif="http://www.serif.com/" style="fill-rule:evenodd;clip-rule:evenodd;stroke-linecap:round;stroke-linejoin:round;stroke-miterlimit:1.5;">
<defs>
<filter id="eye_glow" width="10" height="10" x="-5" y="-5">
<filter id="eye_glow" width="5" height="5" x="-2" y="-2">
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why did this need to change?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It doesn't need to, strictly speaking, it just creates an unnecessarily large box in editors like Inkscape that can make it awkward to work with. I've been trying to trim it down as much as possible.

I think 4 is probably the tightest it can go and still have no impact even at 2048×2048 but that would leave you at x="-1.5" and y="-1.5" so I've just been doing it at 5 width instead. I think I pushed that change to the official neobot SVGs already...

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Makes sense, I'll include it!

in general it's good to describe all your changes in the PR description otherwise there are little surprise nuggets that may not be obvious from the diff

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