search

Advanced-Rocketry / AdvancedRocketry

Space mod for minecraft
http://arwiki.dmodoomsirius.me/
MIT License
216 stars 274 forks source link

Asteroid (genType == 1) Skyboxes do not Recognize the blackHole Tag #1960

Closed lazerous42 closed 3 years ago

lazerous42 commented 3 years ago

[Please fill out the form below and delete the sections in square brackets after reading them]

Version of Advanced Rocketry

AdvancedRocketry-1.12.2-1.7.0-235-universal

Have you verified this is an issue in the latest unstable build

Y ~ but only through code review and not execution

Version of LibVulpes

LibVulpes-1.12.2-0.4.2-75-universal

Version of Minecraft

1.12.2

Does this occur without other mods installed

Yes ~ I didn't run any unreleased code, but I did review the latest revision in the repo. Just looking at the method arguments for AsteroidRenderSky.drawStar(), it doesn't take in a pointer to a StellarBody instance like PlanetRenderSky.drawStar(), so they are handling the star rendering in completely different manners. Nor does the calling AsteroidRenderSky reference StellarBody.isBlackHole() for the elements in the subStars array in AsteroidRenderSky.render()

I hope I explained myself clearly, I do most of my work in Python, with only a little Java. As mentioned, this was only direcly observed in the newest stable release, but located programatically in the HEAD rev

Crash report or log or visualVM (if applicable)

http://pastebin.com is a good place to put them crash reports that are put in the issue itself are hard to read

N/A ~ See steps to reproduce and code analysis above

Description of the problem

Create a planet with genType == 1, orbiting a star with the blackHole flag set. Your skybox will not acknolege the blackHole flag, rendering the stellar object as a normal star

lazerous42 commented 3 years ago

I'm no Java master, but the fix doesn't really look all that hard. I might take a crack at updating this logic and passing you a PR. I can see pretty clearly how to port the star rendering logic from RenderPlanetSky to RenderAsteroidSky. Looks to be a relatively simple port.

github-actions[bot] commented 3 years ago

This issue is stale because it has been open 60 days with no activity. Is this still an issue? If yes please comment that it still is.

voidsong-dragonfly commented 3 years ago

Fixed in 2.0.0