Hardcore Questing Mode [FORGE/FABRIC]

Hardcore Questing Mode [FORGE/FABRIC]

16M Downloads

Array Index Out of Bounds on Client (SMP)

cjm721 opened this issue · 40 comments

commented

HQM Versions: 4.4.0 Originally, Still Occurring on 4.4.4
Forge Versions: 1558 also occurring on 1614

Did all 4 combinations of above.

Server has been running for about three weeks and yesterday about half the server was not able to open their quest book. By the end of the day no one was able to open it. On the client the following error is printed. There is no error message on the server. Clearing the players.dat file fixes it, but don't want to have to reset everyone's quests, and I do not know if this will happen again.

Moving the players.dat to a new world causes the issue which is attached.

https://gist.github.com/cjm721/7a6d06134a5e67930534
players.zip

commented

Reverted back out players dat files to before the issue occurred and it just happened again. Made sure this time no special commands where typed that could of put it in some sort of edit loop or the like.

commented

Copy-pasted from IRC:

<cjm721> The Issue: Being that once the players.dat file exceeds ~500KB the client server custom network communication is getting an index out of bounds on the client. The index it is trying to write to is one past the actual length of the array. The array cannot just be expanded because then it will cause a NPE because it skipped an index. Rather then just doing a very clunky check of moving all the values to the correct index where I don't even know if that will work as that would require clients having a different version of the mod I am trying to solve it all by server side so seeing why when the file is large what is really happening to the QuestData class's hash map that stores all the information.

commented

Complete rewrite of Saving/Loading/NetCode

commented

Well you would think since their mod is so popular that they would bother to fix these problems and not just leave major bugs in the mod

commented

New features and update to 1.8 more important. Been talked about a lot in IRC

commented

haha, forget the new features they need to fix there mod FFS, this mod is full of bugs and they need fixing, they just cant be bothered tbh.. Im not joking this is the last mod pack im using this mod in as its just buggy and have crap support for fixing bugs

commented

Plus these bugs were in the mod even before 1.8 but they still wont fix them

commented

@lorddusk im not joking now you need to fix these bugs as its no point porting this mod to 1.8 that is full of bugs!, its just going to the same mod thats in mc 1.8 with the same bugs that never get fixed?

commented

@MJRLegends Instead of complaining, why not make a pull request with a fix?

@lorddusk keep up the good work.

commented

Because it's not my mod to fix is it

commented

@bookerthegeek from all the work I have put in to learn how the hell this section of code was setup in the first place, of which I could go into a plethora of issues that surround the code base, it take about 8 hours to make a test base for this section and then do the full re-write. Not everyone codes as fast (or slow) as me, nor wants to spend time working on someone else's code base.

Could reduce that time by a lot by not making a testing framework for that section, but then don't know if you are breaking shit outside the scope so not really in a better position to push into master branch.

commented

Any one know a solution to this issue? Because its happened twice now and i've already reset it once???

commented

@MJRLegends did you ever hear about this thing called "Real Life" where
people work 60 hours a week and have next to no free time to fix issues. If
it were up to me id would've been fixed, but sadly I don't have the time to
do so.
I need to make a living to keep myself from losing my stuff.
So I'm very sorry that this is the case and you feel like this.
On Apr 6, 2016 3:11 AM, "MJRLegends" [email protected] wrote:

@lorddusk https://github.com/lorddusk im not joking now you need to fix
these bugs as its no point porting this mod to 1.8 that is full of bugs!,
its just going to the same mod thats in mc 1.8 with the same bugs that
never get fixed?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#174 (comment)

commented

@cjm721 Please don't take anything I've said as a slight against the people actually trying to improve this wonderfully mod. I sorta understand how difficult coding is, and mad props to everybody who is working on this.

commented

Copied my message from IRC to here


@lorddusk, Ok got one of my friends to run start everything up. It does not load old HQM data and causes a NPE https://gist.github.com/cjm721/06102f9d434150a8680cc845ce2a951c But if you wipe out the data it seems to work alright, but won't know if the bug pops up again until have about 1k different people join the server.


Assume this is from the Command Enum added and it expecting it in the binary file.

commented

@way2muchnoise im not trying to be a dick, im just annoyed because the only way to fix this issue is to reset the player quest book data which makes people complain at me....

Please try and understand my view from a mod pack developer and a server owner side

commented

@MJRLegend well the way you are communicating your annoyances is really bad. Modding is a hobby for everyone. This isn't our full time job, if we have to come home from school or work and then start dealing with people saying things like this, that really gets to some people. It is demotivating and annoying, in other words it doesn't get any thing fixed. This code base is old and has lots of code that the original authors of some pieces aren't around any more which makes it hard to untangle all the undocumented code. I have cleaned up a big portion of the code at the end of last year, but lost interest. Updating to 1.8 got a priority as we can just nuke things or change stuff without thinking about backwards compat. The root for this issue is pretty deeply embedded in the code and can't be fixed simply by changing stuff as it might break every HQM pack out there.

commented

@way2muchnoise i understand that but i just think that if it was fixed a long time ago then it wouldnt cause to much issues because there wouldnt of been to many mod pack using it.

commented

@MJRLegends If you ever build a timemachine I'll be glad to go back and fix it for you.

commented

@way2muchnoise haha, will get working on one then

commented

What is actually the issue that is causing this problem as i dont fully get the system in which is controlling this part?

commented

As said it is the network/loading/saving this is all some ByteStream code made by vswe. It works well but it was never designed for large packs or a large amount of players.

commented

Thats what i though, is there not any way of making a workaround to make it work but keep backwards compatibility

commented

Finding a workaround takes tons of time and requires diving into the code. Rewriting is easier, but we'll see where this goes.

commented

@lorddusk I have heard of that yeah but surely you have had some free time over the last years to fix it

commented

@MJRLegends Nobody is forcing you to use this mod. It went open source because of a time issue by the original devs. If you could not be such a dick, that would be nice.

commented

Simple-rewrite is to covert every player data to an individual json file. Load when they join, save when they leave. For players on a team have it point to a different json file with a unique identifier that will be near identical to a player file.

If all of that is done in its own commits I can push it backwards easily as JSON will ignore new fields and if a field has been removed (which from looking at the code nothing has) it would just be marked null.

commented

*Sorry for the close, dog moved my mouse as I clicked -_-

commented

"Simple rewrite"
On Apr 8, 2016 10:38 PM, "CJ Miller" [email protected] wrote:

Simple-rewrite is to covert every player data to an individual json file.
Load when they join, save when they leave. For players on a team have it
point to a different json file with a unique identifier that will be near
identical to a player file.

If all of that is done in its own commits I can push it backwards easily
as JSON will ignore new fields and if a field has been removed (which from
looking at the code nothing has) it would just be marked null.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#174 (comment)

commented

@cjm721 sounds like a good idea but does anyone that knows how the system for it works, to add this feature and make it work with backwards compatibility. Have the time?

commented

@lorddusk I guess simple is a subjective term.

@MJRLegends I know how to make it backwards compatible (could guide any programmer through it) just with the amount of work I have had over the past few months I have not even gotten to work on any my personal projects.

commented

Closed due to no longer supporting 1.7.10, please check if this problem persists in the latest 1.9.4 version. As networking and saving is rewritten for these versions the issue should be resolved.

commented

I still think you should make it for 1.7.10 too

commented

"due to no longer supporting 1.7.10"

commented

I know i can read but i just think you should fix the 1.7.10 since its breaking a lot of packs because of all its bugs

commented

We have decided to move versions and stop supporting older versions. If you want to move it over make fork and PR the changes. But I'm not going to do all of the work for a version I think should be left behind.

commented

In short. if you want to backport the changes from 1.9 to 1.7.10 and make a PR we will release it.

commented

I dont see how it would be hard work since all the work has been done already in thr rewrite of the saving and loading system

commented

Then why don't you do it your self if it isn't hard work.

commented

You have no idea how minecraft code works right?