AntiqueAtlasTeam / AntiqueAtlas

A Minecraft mod that adds a fancy interactive map item.
http://www.minecraftforum.net/topic/2045745-164forge-antique-atlas
Other
251 stars 67 forks source link

Basic port to 1.19.2 #467

Closed sisby-folk closed 5 months ago

sisby-folk commented 1 year ago

Hello again! Been a bit.

1.19.2 is mostly friendly, with a bit of syntax changes.

Two major bumps along the way

Besides that, pretty smooth, had to do a few toolchain bumps but it worked out.

javaw_qX1BExzM4n javaw_F3ESKwi0SS

sisby-folk commented 1 year ago

For your own interest, we also made an itemless fork that we're planning to use for our no-modded-items modpack

tyra314 commented 1 year ago

Thank you for the PR, I'll check it out.

For your own interest, we also made an itemless fork that we're planning to use for our no-modded-items modpack

No need to fork for that. Change itemNeeded config entry to false. If that doesn't work, patches are welcome.

sisby-folk commented 1 year ago

In this specific case (Tinkerer's Quilt) the entire modpack hinges on not adding items to the registry / world at all - it was fun to comb through all of that code though!

sisby-folk commented 1 year ago

Oop! extra thing - we've had this in our modpack for a hot minute and it seems like tile choosing for nether isn't working, so we may have broken that accidentally with a sensible-seeming replacement that didn't quite cut it

On Tue, 16 May 2023, 4:52 am tyra314, @.***> wrote:

Thank you for the PR, I'll check it out.

For your own interest, we also made an itemless fork https://github.com/sisby-folk/AntiqueAtlas/compare/1.19...sisby-folk:AntiqueAtlas:1.19_itemless that we're planning to use for our no-modded-items modpack

No need to fork for that. Change itemNeeded config entry to false. If that doesn't work, patches are welcome.

— Reply to this email directly, view it on GitHub https://github.com/AntiqueAtlasTeam/AntiqueAtlas/pull/467#issuecomment-1548385753, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANJ34KNG2HGCNXP7T5S3KJ3XGJ3N7ANCNFSM6AAAAAAYB2PJVY . You are receiving this because you authored the thread.Message ID: @.***>

sisby-folk commented 1 year ago

False alarm! I actually had yttr installed, which replaces nether biomes in some way - they show fine with vanilla!

sisby-folk commented 1 year ago

More notes as we've gone along.

java.lang.NullPointerException: Cannot invoke "net.minecraft.class_1657.method_5770()" because the return value of "dev.architectury.networking.NetworkManager$PacketContext.getPlayer()" is null

   at hunternif.mc.impl.atlas.network.packet.s2c.play.MapDataS2CPacket.lambda$apply$0(MapDataS2CPacket.java:42) ~[transformed-mod-antiqueatlas.i0:0/:?]
tyra314 commented 1 year ago
* While debugging, we've occasionally experienced S2C tile packets fail, causing filled atlases to get stuck blank. Seems timing and architectury related, but we can't reproduce it consistently. Yet to see it in a production environment like our pack.

Yeah, I've seen this shit as well. Really annoying. Didn´t notice though that it only occurred in dev environments. I could have sworn that I had a bug report for that.

sisby-folk commented 1 year ago

@tyra314 on the "stuff failing to send to the client thing" - I think you might be right and it might be happening on prod too? I thought it was just a mod conflict, but I'm occasionally getting netty exceptions on both the client and server, and I've been experiencing a bug (which I thought was unrelated) where markers in the nether would get completely wiped. Not too sure there!

Here's the log bug that makes maps totally blank, though:

image

I think it happens consistently if you log out / save while holding an atlas in your hand. That would imply the game is checking for atlases before the player is actually initialized properly.

tyra314 commented 1 year ago

I think it happens consistently if you log out / save while holding an atlas in your hand. That would imply the game is checking for atlases before the player is actually initialized properly.

Well, at least that is a good starting point to reproduce the issue.

Daenyth commented 11 months ago

Is there any way I can help with this?

sisby-folk commented 9 months ago

@tyra314 hi hello! I'm at the point where I'm thinking about poking 1.20.1 with a stick, any further thoughts on this one? (besides not being able to display underground biomes, naturally)

Daniilnm commented 9 months ago

hello, I want to ask how the work is progressing, there have been no changes in this branch for a long time

Daniilnm commented 9 months ago

@tyra314 hi hello! I'm at the point where I'm thinking about poking 1.20.1 with a stick, any further thoughts on this one? (besides not being able to display underground biomes, naturally)

.

tadhgmister commented 8 months ago

@sisby-folk, I tried using the artifact from this pull request but ran into an error:

java.lang.IllegalAccessError: 
class hunternif.mc.impl.atlas.client.TileTextureMap tried to access private method
'boolean hunternif.mc.impl.atlas.client.forge.TileTextureMapImpl.biomeIsJungle(net.minecraft.core.Holder)'

And looking at the file, biomeIsJungle is the only private method of the lot, every other one is public and there was a commit 3 months ago where you fixed visibility for several other methods but jungle was left out.

It crashes as soon as I try to load a world using forge, I'm not sure how to suggest an edit, do you think you could fix that in the correct place to change it? Thanks :)

TheOverpassArsonist commented 8 months ago

What's the status of this? As far as I can tell issue that's been mentioned has already had commits addressing it/them, and every item in the requested changes by tyra show as resolved. Is this done and it's just a matter of publishing it, or are there remaining issues? (for reference, not a mod developer myself, but I checked in to see what the progress was at and I noticed that it seems like everything that's been mentioned has been addressed)

The one thing that github is citing as an issue is the requested changes, but as far as I can tell all of the requested changes have been made. On sept 11th this comment was made implying it was solid about a month and a half ago and (aside from one private/public mixup) it doesn't look like any other issues have cropped up.

Not trying to poke or prod for some ETA/date just curious as to the general state this PR is in since it appears as though every fix that was requested/needed has been made already.

sisby-folk commented 6 months ago

Hey hello! @tyra314 just a heads up that we're planning to put our itemless forks of this PR (and the 1.20.1 one) on modrinth within a week or so. We weren't originally planning on it, but someone else went and did it without asking or notifying us, and we'd like to avoid that happening a second time.

We'll be forgoing any monetization on modrinth and crediting clearly to suit the cc-nc-by-sa textures.

Naturally, let us know if there are any issues, and as always you're still perfectly welcome to adapt any of our changes in these PRs into the main mod and MR project. Cheers!