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

Created AsciiEffect (to improve), Ascii Content Page and Ascii Demo (to fix) #684

Merged
merged 2 commits into from
Feb 15, 2025

Conversation

LucaArgentieri
Copy link

@LucaArgentieri LucaArgentieri commented Feb 15, 2025

Related issue: #683

Description

Created:

  • AsciiEffect (to improve), I took the effect from there and transferred it to the new file, so you can do refactor and figure out what you can add.
  • Ascii Content Page, inserted the preview scene and put in alphabetical order.
  • Ascii Demo (to fix), I created the file also for the demo but maybe it’s a bit early, in case it is not useful at the moment you can easily delete it.

@vanruesc vanruesc merged commit c93a856 into pmndrs:dev Feb 15, 2025
@vanruesc
Copy link
Member

Thanks! I'll take it from here.

@LucaArgentieri
Copy link
Author

If you have contribution guidelines I can try to improve the code and make it consistent with the library.

I can also try to understand how the other effects are managed, I will try to contribute as I can

@vanruesc
Copy link
Member

vanruesc commented Feb 16, 2025

The contribution guide can be found here.

There's not much that needs to be changed here apart from some linting and refactoring. I'm short on time and it's faster for me to do the clean up myself, but thanks for offering help. Also, optimizing the shader requires active debugging which isn't really doable through PR comments.

You can check out the concepts in v7 through the existing effects and then try your hand at porting this effect to v7 once I'm done with the refactoring. I'll create a new ticket for that task.

@vanruesc vanruesc mentioned this pull request Feb 17, 2025
4 tasks
@vanruesc
Copy link
Member

#685

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