search

MCHPR / MCHPRS

A multithreaded Minecraft server built for redstone.
MIT License
1.55k stars 68 forks source link

Regression in sammyuri's Minecraft in Minecraft #125

Open ghost opened 10 months ago

ghost commented 10 months ago

https://github.com/MCHPR/MCHPRS/commit/766301de9906ab0c743695446ee2104791e75102

git checkout 766301de9906ab0c743695446ee2104791e75102 .

Everything works: works

https://github.com/MCHPR/MCHPRS/commit/57a650b273c674e13f5de82c12d8e38b7c6acb31

git checkout 57a650b273c674e13f5de82c12d8e38b7c6acb31 .

Rendering is broken, the camera can still move though:

broken

Edit: to clarify, it is broken since then, even in the latest commit

Edit 2: simply reverting the commit does nothing, the problem is somewhere else, sorry for the confusion

Edit 3: https://github.com/MCHPR/MCHPRS/commit/c3bc9f502d00c2690d24884835fd4b9cb69e5d97 is the last (?) that really works. This time I didn't forget to use git reset --hard

BramOtte commented 7 months ago

I do not know how I haven't noticed this regressing before but I just got a similar issue with the chungus2 mandlebrot benchmark which seams to have started at the exact same point in git history. Since it took so long to spot I fear rust changed something in the compiler or our testing has simply not been thorough enough. The commit where everything seams to break is described as a fix, assuming this is not some weird change in the rust compiler, I wonder if chungus2 needs that bug to function or if it was not implemented correctly. If the commit that broke chungus2 does work as intended it might be best to just have some configuration flag to compile mchprs to work with chungus2 or chungus2 itself can be fixed.

StackDoubleFlow commented 7 months ago

I'm fairly certain the problem is in the screen hardware that might depend on a specific mchprs inaccuracy with repeater updates that has now been fixed (I'm assuming this is the commit you're talking about). I don't intend on having a configuration flag to work around this, so the redstone will also have to be fixed.

ghost commented 6 months ago

I don't intend on having a configuration flag to work around this, so the redstone will also have to be fixed.

Could you please reconsider this? That's probably the greatest redstone build out there, it would be nice if it continued to work on the most recent versions of MCHPRS with all the new improvements. It would even be ok for me if the configuration flag had to be set before compilation

IUM259 commented 2 weeks ago

One problem that MiM has is the multipliers, there are several in AMOGUS GPU, please check this