TIS-3D icon indicating copy to clipboard operation
TIS-3D copied to clipboard

Ported to Fabirc 1.17

Open lucsoft opened this issue 3 years ago • 14 comments

Only Document.render is still missing

Then its merge ready and can directly be published for the public

lucsoft avatar Jun 05 '21 20:06 lucsoft

Thanks for the review! Didn't take the time yet, because it was still marked as draft.

FTR, I intend to try and merge the Forge branch into the Fabric one on 1.17 to get the two more in sync again; should be less painful now that Forge also uses the Mojang mappings for class names. Just not sure when I'll make the time for that :P

fnuecke avatar Aug 09 '21 11:08 fnuecke

Ah my bad, I must have overlooked that it's still a draft.

Sturmlilie avatar Aug 09 '21 13:08 Sturmlilie

Is there any way I (one who doesn't have modding experience) can help finish this PR?

KyGost avatar Sep 18 '21 01:09 KyGost

it would be better to migrate this first https://github.com/MightyPirates/MarkdownManual and then this pr can go forword

(Pull requests are open for my fork)

lucsoft avatar Sep 18 '21 08:09 lucsoft

it would be better to migrate this first MightyPirates/MarkdownManual and then this pr can go forword

(Pull requests are open for my fork)

I actually ported the documentation to Patchouli once, but it turns out that the book format doesn't really work well. (Text is very large and there are no code blocks)

Jummit avatar Sep 18 '21 08:09 Jummit

So... Looking at the current state of this PR, looks like what remains is just moving to a different library for the manual? Using the manual in the current build shows an opening page and GUI but clicking for other pages has no effect. Further, the console is spammed with errors.

I'll have a go at migrating over to the API linked (assuming I have some time) and submit a PR to the fork.

KyGost avatar Sep 20 '21 04:09 KyGost

I did start porting the extracted manual lib to fabric, not quite done yet. I might get to wrap that up next week.

Ideally this branch could be brought back closer to the forge branch in general (there were some changes to the module rendering API I'm pretty happy with), but that could come later.

fnuecke avatar Sep 20 '21 07:09 fnuecke

Ah, is there any way I can help to speed that up?

KyGost avatar Sep 20 '21 08:09 KyGost

Found a bug: Right clicking ROM module shows: 2021-09-20_19 50 18 2021-09-20_19 50 22

Nothing special logged in console

KyGost avatar Sep 20 '21 09:09 KyGost

Oh yeah the atlas is wrong probably a quick fix if you look at my git history

(This was very Common while porting)

lucsoft avatar Sep 20 '21 10:09 lucsoft

Ah, is there any way I can help to speed that up?

Thanks, much appreciated, but I'm afraid not. I'm currently on vacation and didn't push before leaving 👀

fnuecke avatar Sep 20 '21 16:09 fnuecke

@fnuecke sorry to bother you, I'm also interested in a fabric port of MarkdownManual for another mod, was just curious to hear how long you're on vacation for? (So I know whether to look for another solution or wait it out)

KyGost avatar Sep 28 '21 09:09 KyGost

@lucsoft

Oh yeah the atlas is wrong probably a quick fix if you look at my git history

(This was very Common while porting)

I've got absolutely no idea where to start looking. Any chance you can give me more of a pointer? Trying to figure out what's going wrong and what needs to change. My minimal knowledge leads me to suspect it is failing to grab the texture.

This is really throwing me off and seems like a gross API:

client.getTextureManager().bindTexture(Textures.LOCATION_GUI_MEMORY);
drawTexture(matrices, guiX, guiY, 0, 0, GUI_WIDTH, GUI_HEIGHT);

We aren't saving the texture we've bound to then draw it, we are just binding into the abyss and trusting that it is remembered and presumably then trying to draw it??

KyGost avatar Oct 05 '21 07:10 KyGost

Figured it out, still think it is yuck. Needed this stuff:

...
RenderSystem.setShaderTexture(0, Textures.LOCATION_GUI_MEMORY);
...

KyGost avatar Oct 05 '21 08:10 KyGost

Well, this is awkward; I'm so so terribly sorry, but I completely forgot this PR existed. I migrated the project to Architectury over the last few days, and just saw this now :/ Again, sorry not following up on this one, but I'm afraid I'd rather stick to the current MC versions now, so I'll close this one.

fnuecke avatar Jan 06 '23 14:01 fnuecke

You moved to Architectury? NICE!

lucsoft avatar Jan 06 '23 14:01 lucsoft