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

Implement replacetextEx_char, reimplement Splittext to match new 515 behavior #1921

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

MyNameIsRomayne
Copy link

Very simple one-liner piggybacking off jointext and spltitext, with an update to Splittext so that it functions properly.

Basically, splittext has been updated in 515 to include the start and end of the string, so the current implementation of Splittext now remembers that and pre/appends those parts to the first/last elements.

The splittext unit tests were updated based off the output from DM for 515:
image

Originally this was just going to be a one-liner for replacetextEx_char, but after being informed of the changes needed to Splittext I decided to go ahead and do that too, so that this works.

@boring-cyborg boring-cyborg bot added Compiler Involves the OpenDream compiler Runtime Involves the OpenDream server/runtime labels Aug 13, 2024
@MyNameIsRomayne MyNameIsRomayne marked this pull request as draft August 13, 2024 21:51
@MyNameIsRomayne
Copy link
Author

Drafting due to a new test I added failing, going to resolve that (Empty strings should be splitting on every char, but instead dont split at all)

@github-actions github-actions bot added size/L and removed size/M labels Aug 14, 2024
@MyNameIsRomayne MyNameIsRomayne marked this pull request as ready for review August 14, 2024 21:47
@MyNameIsRomayne
Copy link
Author

MyNameIsRomayne commented Aug 14, 2024

image
Reopening because I am genuinely unable to fix this without worsening the code quality beyond the review-necessary state it is in. Please advise on this

Copy link

github-actions bot commented Sep 9, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@amylizzle
Copy link
Contributor

Maybe mark as draft if it's not ready for merge

@MyNameIsRomayne MyNameIsRomayne marked this pull request as draft December 5, 2024 15:57
@MyNameIsRomayne
Copy link
Author

Yeah these haven't been worked on for a while, I'm not sure if I'll be able to get to them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compiler Involves the OpenDream compiler Merge Conflict Runtime Involves the OpenDream server/runtime size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants