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

PenguinsOnIce-saumil #697

Closed
wants to merge 19 commits into from
Closed

PenguinsOnIce-saumil #697

wants to merge 19 commits into from

Conversation

saumilthecode
Copy link
Contributor

So this is my third attempt at attaining a blot :D
so in this attempt I expounded on the last attempt by making the penguin draw abit more thicker so it looks more like a penguin. Other then that I also made the "cracks" in the background look more like cracks so its more like real penguins on ice :)
thanks for your time!

@saumilthecode
Copy link
Contributor Author

yay

Copy link
Member

@maxwofford maxwofford left a comment

Choose a reason for hiding this comment

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

This is looking awesome! A couple things to fix though:

  • currently it's drawing off the page– the machine can't do that!
  • the penguins are drawn in white, but the machine can't draw in white! you should probably leave that unshadded
  • There are some strange lines going up from the bottom of the page to the penguins. can you fix that?
  • the snapshot file names shouldn't have capitals or spaces!

@saumilthecode saumilthecode requested a review from maxwofford July 15, 2024 09:14
@saumilthecode
Copy link
Contributor Author

Hello! @maxwofford could you take a look now?
I changed the snapshot file names, redid the colour the penguins are drawn in and made it respect the page boundaries! For some reason I can't manage to stop the trailing lines but I tried to and reduced the number of trails it makes

Copy link
Member

@maxwofford maxwofford left a comment

Choose a reason for hiding this comment

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

Nice work! I'm still seeing the strange lines coming up from the bottom, but it's a little artistic looking!

I don't see the color changes though:

Screenshot 2024-07-15 at 17 33 31

@saumilthecode
Copy link
Contributor Author

Hello! @maxwofford
so after experimenting, I decided a plain black belly looks the best. (The blank looks a bit weird cause of the way it reders)
I hope this gets approved!

@saumilthecode saumilthecode requested a review from maxwofford July 16, 2024 14:55
@maxwofford maxwofford changed the title Add PenguinsOnIce-saumil PenguinsOnIce-saumil Jul 18, 2024
Copy link
Member

@maxwofford maxwofford left a comment

Choose a reason for hiding this comment

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

This is getting closer! You still have some of those trailing lines, and I think the smaller penguins have their lines so close together it'll cause the pen to tear through the page.
Screenshot 2024-07-18 at 11 34 45
Screenshot 2024-07-18 at 11 34 48

@saumilthecode
Copy link
Contributor Author

hello! @maxwofford I changed it so smaller penguins don't have that issue anymore. So I left the trailing lines to remain still since it looks nice for some pieces!

Copy link
Member

@BrightTheBackpack BrightTheBackpack left a comment

Choose a reason for hiding this comment

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

I still think it would be nice to remove those lines(completely up to you though)
I found the cause of the line was that sometimes y2 would be NaN, so a quick patch fix would be to add someething like this around line 72:
let [x2, y2] = intersections[k + 1];
if(isNaN(y2)){
y2 = y1
}

@saumilthecode
Copy link
Contributor Author

Hi! @BrightTheBackpack I finally figured out how to remove the lines thanks to your help!

would this be enough to get a blot?

thanks for your time!

Copy link
Member

@BrightTheBackpack BrightTheBackpack left a comment

Choose a reason for hiding this comment

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

I think this gtg! Max will be around in about a week to do a final review

@saumilthecode
Copy link
Contributor Author

yay okie!

Copy link
Member

@maxwofford maxwofford left a comment

Choose a reason for hiding this comment

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

This still isn't quite at the level you need to get to for a blot. Keep working on it and plz tag me once you're sure you've done something that'd hit the quality of the gallery

@saumilthecode
Copy link
Contributor Author

Hello! @maxwofford I made large changes to how they look now!

I gave them eyes and larger flippers (hahahhaha flipper zero reference)

also, it is intended to not fill in the area since it would use up a (b)lot of link

hope this hits the bar!

@saumilthecode
Copy link
Contributor Author

Hello! @maxwofford pinging again since a week has passed (I understand yall are really really busy) but could yall squeeze a look at my art in (I've been working on this on and off for nearly 2 months now (not counting the 2 other PR's which got closed for not meeting the criteria)

Thanks! :D

Copy link
Member

@qcoral qcoral left a comment

Choose a reason for hiding this comment

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

Hey! Sorry reviewing took so long. Can you add a variable at the top that disables all colour/varying thickness lines? That would indicate what it looks like when it's actually drawn on Blot!

@saumilthecode
Copy link
Contributor Author

ok! changing now!

Copy link

vercel bot commented Aug 28, 2024

@saumilthecode is attempting to deploy a commit to the Hack Club Team on Vercel.

A member of the Team first needs to authorize it.

@saumilthecode
Copy link
Contributor Author

heyo @qcoral made the changes !1

@saumilthecode saumilthecode requested a review from qcoral August 29, 2024 00:12
@EmperorNumerius EmperorNumerius self-assigned this Sep 10, 2024
@EmperorNumerius
Copy link
Collaborator

I think this is there
Alex will review it in sometime but i think you are there

@saumilthecode
Copy link
Contributor Author

heartbreaking that its been 9 weeks now with nothing in the last 3 😔

@EmperorNumerius
Copy link
Collaborator

Also I totally see where you are from but do remember tha all of us are in highschool/just left
im really busy with colllege apps and other school work, and alex is working on hackpad and was at htn until today lol

@saumilthecode
Copy link
Contributor Author

hullo!
would there be any way to get this fast tracked :p
I would really appreciate it if we managed to get a blot for our club day (3rd of October) :D
(if not then I guess we'll have to make do with printouts and videos)

@saumilthecode
Copy link
Contributor Author

this should be it all now!

@saumilthecode
Copy link
Contributor Author

added iceburgs + scarfs + made em rotate + made em more "penguin-like"!!!11

Copy link
Member

@qcoral qcoral left a comment

Choose a reason for hiding this comment

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

nice job on this! I think it could still use more polish though. Beyond that - your program filename needs to be index.js. Could you change that?

@qcoral
Copy link
Member

qcoral commented Oct 15, 2024

Closing this for now - make a new PR with your changes!

@qcoral qcoral closed this Oct 15, 2024
@saumilthecode saumilthecode mentioned this pull request Oct 23, 2024
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants