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

1.19 #453

Closed LangYueMc closed 1 year ago

LangYueMc commented 1 year ago

The biome category has been deleted by mojang, so the mod-compatible code is invalid, and can only be customized with texture packs.

tyra314 commented 1 year ago

Thank you for your work, but as is, it won't help much. I haven't updated to 1.19 yet because it's hard or anything, but because I want to iron out some of the existing bugs first. Also, I don't want to skimp out on the fixes for the 1.18 version. I'd gladly accept the Iris bug fix and the third-person render as individual PRs. The 1.19 patch has to wait for now.

LangYueMc commented 1 year ago

Ok, I will close it. The iris compatible code is not very elegant. It belongs to the fact that iris broke the rendering mechanism. My solution is only temporarily compatible. You can copy it directly. As for the third-person rendering, I can resubmit the PR later, but I think someone has already done this, although I have some changes.

artemisSystem commented 1 year ago

I want to iron out some of the existing bugs first

@tyra314 can you say something about what these bugs are? I'd love to contribute to fix things if it means we can have this wonderful mod on 1.19

tyra314 commented 1 year ago

Bugs are tracked in the issues. The biggest one is probably the village detection based on the jigsaw generators. They used to be only used for villages, but not anymore. Hence requiring a complete overhaul of the system in general.

Besides that, this pr isn't finished. It hasn't tackled the new biomes.

I understand the urge to have every mod at the newest minecraft version, but please also understand that not everything helps. Fixing bugs is more important than having a broken mod for every version of minecraft. And instead of spending my limited time on actually modding and fixing stuff, I have to deal with overburdening requests all the time.

Also, I don't know what to think about your fork on curse forge yet. Is it legal? Maybe. Is it derailing my time? Yes. Is it demotivating? Definitely.

Vazkii commented 1 year ago

Also, I don't know what to think about your fork on curse forge yet. Is it legal? Maybe. Is it derailing my time? Yes. Is it demotivating? Definitely.

My apologies, I was the one who requested it be made based on this branch so we could use it in our pack for our server. We're gonna have it taken down soon so it's not a thorn on your side.

artemisSystem commented 1 year ago

Also, I don't know what to think about your fork on curse forge yet. Is it legal? Maybe. Is it derailing my time? Yes. Is it demotivating? Definitely.

Apologies. When vazkii asked me to publish it on curseforge, I just did it without much more thought, other than making sure it was allowed within the license. That's my bad, I definietly should have thought through it more through. I've deleted the project again, sorry.

Bugs are tracked in the issues. The biggest one is probably the village detection based on the jigsaw generators. They used to be only used for villages, but not anymore. Hence requiring a complete overhaul of the system in general.

I might be interested in taking a look at this bug sometime soon. However, I can't find the issue for it? Can you give a link to the right one so I can have a look at it?

LangYueMc commented 1 year ago

Bugs are tracked in the issues. The biggest one is probably the village detection based on the jigsaw generators. They used to be only used for villages, but not anymore. Hence requiring a complete overhaul of the system in general.

This seems to be fixed by me, or there is no bug in itself, just maybe a village is considered multiple (maybe the reason I installed other mods)

Besides that, this pr isn't finished. It hasn't tackled the new biomes.

The bug of the new biome seems to be unsolvable, because the API modification of minecraft makes it impossible to recognize the biome type (maybe I did not find the modification code of minecraft), unless a set of biome API is implemented, and the biome mod is based on this API, but I think it is difficult, so I am more inclined to let the mod package author write the datapack to ensure compatibility.

LangYueMc commented 1 year ago

Also, this PR has been closed for quite a while, how did you find it? XD

artemisSystem commented 1 year ago

The bug of the new biome seems to be unsolvable, because the API modification of minecraft makes it impossible to recognize the biome type (maybe I did not find the modification code of minecraft), unless a set of biome API is implemented, and the biome mod is based on this API, but I think it is difficult, so I am more inclined to let the mod package author write the datapack to ensure compatibility.

In newer minecraft versions you're supposed to use biome tags instead of biome categories. Tags have sort of become the new "biome API". Vanilla has some biome tags, and then forge and fabric also extend the system with their own. See here for more info https://gist.github.com/TelepathicGrunt/b768ce904baa4598b21c3ca42f137f23

Also, this PR has been closed for quite a while, how did you find it? XD

I looked at the network graph in the insights tab and found your fork there. I also happened to randomly stumble across it when i clicked the closed PRs button

tyra314 commented 1 year ago

My apologies, I was the one who requested it be made based on this branch so we could use it in our pack for our server. We're gonna have it taken down soon so it's not a thorn on your side.

Apologies. When vazkii asked me to publish it on curseforge, I just did it without much more thought, other than making sure it was allowed within the license. That's my bad, I definietly should have thought through it more through. I've deleted the project again, sorry.

Thank you. No hard feelings. To elaborate more on my doubts regarding the legality, while the source code of AA is GPL licensed, the default textures shipped with it are not. Hence, putting up the compiled mod is totally fine, but depending on the interpretation of "commercial" it might be problematic to ship the default AA-textures on curseforge for forks. As Hunternif originally create the AA project on CF, I'm rolling with it, but I'm not even sure if that is technically legal. This is also partly the reason, why I haven't put up the updated Golriths textures resource-pack on CF.

I might be interested in taking a look at this bug sometime soon. However, I can't find the issue for it? Can you give a link to the right one so I can have a look at it?

443 is a symptom of that detection code insufficiency. The current implementation uses simple string contains checks for jigsaw structures. As other mods moved to use jigsaw as well, just looking if "house" is part of a jigsaw piece id doesn't cut it anymore. What needs to be done is me stop being lazy and cutting corners, I have to look up all pieces there are in vanilla and put together a data-pack representing the necessary data. Then there needs to be a generic implementation that can read such data-packs and set the tiles depending on that.

The bug of the new biome seems to be unsolvable, because the API modification of minecraft makes it impossible to recognize the biome type (maybe I did not find the modification code of minecraft), unless a set of biome API is implemented, and the biome mod is based on this API, but I think it is difficult, so I am more inclined to let the mod package author write the datapack to ensure compatibility.

That is an issue and was in plain-sight for me, but not what I was referring to. What I actually meant was support for the new biomes: mangrove and swamps. Also I could swear there's a new birch forest, or did they delayed that in the end? These need extensions to the default resource pack and may even need some additional textures.

tyra314 commented 1 year ago

In newer minecraft versions you're supposed to use biome tags instead of biome categories. Tags have sort of become the new "biome API". Vanilla has some biome tags, and then forge and fabric also extend the system with their own. See here for more info https://gist.github.com/TelepathicGrunt/b768ce904baa4598b21c3ca42f137f23

Thank you very much for sharing this. It looks really helpful.