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

Pen+ V6 #535

Merged
merged 14 commits into from
Nov 6, 2023
Merged

Pen+ V6 #535

merged 14 commits into from
Nov 6, 2023

Conversation

David-Orangemoon
Copy link
Contributor

@David-Orangemoon David-Orangemoon commented Jun 5, 2023

A massive rewrite of the extension no longer compatible so it is a different file to allow use of the old one.

Maybe closes #170

Closes #226

@David-Orangemoon
Copy link
Contributor Author

image_2023-06-27_211510576
The roster of the update.

@yuri-kiss
Copy link
Contributor

did you ever fix the image stamping issue

@David-Orangemoon
Copy link
Contributor Author

did you ever fix the image stamping issue

?

@yuri-kiss
Copy link
Contributor

yuri-kiss commented Jun 28, 2023

did you ever fix the image stamping issue

?

image

image

@David-Orangemoon
Copy link
Contributor Author

did you ever fix the image stamping issue

?

image

image

Oh yeah this version of pen+ works completely differently.

@yuri-kiss
Copy link
Contributor

Ya!

@David-Orangemoon
Copy link
Contributor Author

Celebrating 1 month in pull request LIMBO!

@TheShovel
Copy link
Contributor

Woooo! Let's go!

@PuzzlingGGG
Copy link

yay!!!!

@David-Orangemoon
Copy link
Contributor Author

Also updated extension builder

This was referenced Jul 17, 2023
@tigercoding56
Copy link

can you add a way to render polygons from a list of 2d coordinates ?

@David-Orangemoon
Copy link
Contributor Author

David-Orangemoon commented Jul 23, 2023 via email

@tigercoding56
Copy link

You can just iterate through the list.

On Sun, Jul 23, 2023, 5:49 PM tigercoding56 @.> wrote: can you add a way to render polygons from a list of 2d coordinates ? — Reply to this email directly, view it on GitHub <#535 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASKLQSJHKJDW3FQYADH42TLXRWL5BANCNFSM6AAAAAAY2NYNTY . You are receiving this because you authored the thread.Message ID: @.>

that is slow AF , not to mention how slow filling it in is

@David-Orangemoon
Copy link
Contributor Author

You can just iterate through the list.

On Sun, Jul 23, 2023, 5:49 PM tigercoding56 @.> wrote: can you add a way to render polygons from a list of 2d coordinates ? — Reply to this email directly, view it on GitHub <#535 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASKLQSJHKJDW3FQYADH42TLXRWL5BANCNFSM6AAAAAAY2NYNTY . You are receiving this because you authored the thread.Message ID: _@**.**_>

that is slow AF , not to mention how slow filling it in is

How are you filling them in?

@David-Orangemoon
Copy link
Contributor Author

You can just iterate through the list.

On Sun, Jul 23, 2023, 5:49 PM tigercoding56 @.> wrote: can you add a way to render polygons from a list of 2d coordinates ? — Reply to this email directly, view it on GitHub <#535 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASKLQSJHKJDW3FQYADH42TLXRWL5BANCNFSM6AAAAAAY2NYNTY . You are receiving this because you authored the thread.Message ID: _@**.**_>

that is slow AF , not to mention how slow filling it in is

Also what version of pen+ are you running?

@skyistumbling
Copy link

we need this now

@LilyMakesThings
Copy link
Contributor

thank you for fixing all menus

@David-Orangemoon
Copy link
Contributor Author

thank you for fixing all menus

get good

Copy link
Contributor

@Samq64 Samq64 left a comment

Choose a reason for hiding this comment

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

The docs allow using scratchbocks so all the images and CSS aren't necessary anymore.

I can't fully review this because it's a lot of code but also because I'm not very familiar with WebGL.

extensions/obviousAlexC/SensingPlus.js Outdated Show resolved Hide resolved
extensions/obviousAlexC/penPlus.js Outdated Show resolved Hide resolved
extensions/obviousAlexC/penPlus.js Show resolved Hide resolved
extensions/obviousAlexC/penPlus.js Show resolved Hide resolved
extensions/obviousAlexC/penPlus.js Outdated Show resolved Hide resolved
extensions/obviousAlexC/penPlus.js Outdated Show resolved Hide resolved
extensions/obviousAlexC/penPlus.js Show resolved Hide resolved
extensions/obviousAlexC/penPlus.js Show resolved Hide resolved
@David-Orangemoon
Copy link
Contributor Author

David-Orangemoon commented Aug 19, 2023 via email

@David-Orangemoon
Copy link
Contributor Author

David-Orangemoon commented Aug 19, 2023 via email

Copy link
Member

@GarboMuffin GarboMuffin left a comment

Choose a reason for hiding this comment

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

Realistically I will never be able to give this a full proper review

I left some comments

Some other feedback:

  • scratch-render has a "render regions" system that they use to make sure the right buffers are bound at the right time and not a moment sooner. I think you should try using that system instead of haphazard buffer binding
  • you might want to consider just completely replacing the pen skin honestly. As-is this extension's existence means that PenSkin.js basically can't be touched anymore. That's not ideal.

extensions/obviousAlexC/penPlus.js Outdated Show resolved Hide resolved
extensions/obviousAlexC/penPlus.js Show resolved Hide resolved
extensions/obviousAlexC/penPlus.js Show resolved Hide resolved
@GarboMuffin
Copy link
Member

also, lowercase "Get pen square's [Width v]"

@David-Orangemoon
Copy link
Contributor Author

Realistically I will never be able to give this a full proper review

I left some comments

Some other feedback:

  • scratch-render has a "render regions" system that they use to make sure the right buffers are bound at the right time and not a moment sooner. I think you should try using that system instead of haphazard buffer binding
  • you might want to consider just completely replacing the pen skin honestly. As-is this extension's existence means that PenSkin.js basically can't be touched anymore. That's not ideal.

The thing with replacing penSkin.js is that it would no longer be compatiable with the scratch pen which I'm trying to make it compatible with.

@David-Orangemoon
Copy link
Contributor Author

also there's a menu with acceptReporters: false to reconsider

Sorry that is an artifact from the ExtensionBuilder compiler I'll fix that.

@David-Orangemoon
Copy link
Contributor Author

Realistically I will never be able to give this a full proper review

I left some comments

Some other feedback:

  • scratch-render has a "render regions" system that they use to make sure the right buffers are bound at the right time and not a moment sooner. I think you should try using that system instead of haphazard buffer binding
  • you might want to consider just completely replacing the pen skin honestly. As-is this extension's existence means that PenSkin.js basically can't be touched anymore. That's not ideal.

Garbo do you mean draw regions when you say render regions?

@GarboMuffin
Copy link
Member

do you consider this done?

@GarboMuffin
Copy link
Member

Aside from the one minor gripe ESLint has

@David-Orangemoon
Copy link
Contributor Author

David-Orangemoon commented Aug 26, 2023 via email

@David-Orangemoon
Copy link
Contributor Author

Ok I need to get home and I will fix valuetoset errors

@NexusKitten

This comment was marked as spam.

@LilyMakesThings
Copy link
Contributor

Bro's magic, he actually passed eslint

considering style errors don't exist anymore, so long as your code actually does what it's supposed to, it should be fine now

@David-Orangemoon
Copy link
Contributor Author

Fixed minor depth buffer mistake

@AlexSchoolOH
Copy link
Contributor

I give up

@CubesterYT
Copy link
Collaborator

What 😭😭😭

@NexusKitten

This comment was marked as spam.

@AlexSchoolOH
Copy link
Contributor

What 😭😭😭

I'm not dealing with turbowarp anymore.

@GarboMuffin
Copy link
Member

I get it, I really do

I will get this over the finish line for you one day, thanks for your contributions thus far

@David-Orangemoon
Copy link
Contributor Author

Keeping this and Sensing+ V3 up due to them still having the possibility of being merged

@NexusKitten

This comment was marked as spam.

@GarboMuffin GarboMuffin merged commit 182580b into TurboWarp:master Nov 6, 2023
3 checks passed
@CubesterYT
Copy link
Collaborator

LETS GOOOOO!!

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