MapleScala

[Minike]

Also, please remove attribution to me on this code that I did not write

Actually according to the MIT license I should mention the original license on anything that would be a copy (Possibly this is just my misunderstanding of what a 'copy' could be considered to be)

The above copyright notice and this permission notice shall be included in
all copies or substantial portions of the Software.

Thank you, tone police.

The whole point why I said this was because my expectations of any reply would be such as your later, not like the first one.

Anyways, thank you for this reply, I will use as a reference to create my own reader. (as you apparently do not like the fact your public source is re-used)

[retep998]

https://github.com/Yaminike/MapleSca....scala#L33-L35

    using(getClass.getResourceAsStream("/XML/Etc.nx.gz"))(stream => {
      bytes = GzipParser.readGzip(stream)
    })

Really? You're loading compressed NX files?
I find it hilarious how people are using the NX file format.
NX was designed first and foremost for me, for my C++/Rust, so that I can memory map the file and access the data directory with almost no loading and without using up significant amounts of memory.
Meanwhile here you are compressing your NX files so that not only do you have all the loaded NX data in memory, but you also have the decompressed file itself in memory.
The set of NX files for servers (so no bitmaps or sounds) was 451 MB back in GMS v141. Do you really want to throw that much memory away, plus the CPU time to decompress that, just for the sake of saving a bit of hard drive space?

[aaronweiss]

Actually according to the MIT license I should mention the original license on anything that would be a copy (Possibly this is just my misunderstanding of what a 'copy' could be considered to be)

Yes, the license says to maintain the license. I still request that you change the name of the library, as while that is basically copy+paste of my code, I don't want my library to be associated with such a lousy "port."

Anyways, thank you for this reply, I will use as a reference to create my own reader.

Or you could just use the library to begin with and submit a PR if you want to make changes.

(as you apparently do not like the fact your public source is re-used)

The snark is real. The issue isn't that it's being re-used. The issue is that you're not actually using it. You "ported" it to Scala by taking the exact same code and rewriting it with Scala syntax.

[Minike]

https://github.com/Yaminike/MapleSca....scala#L33-L35

    using(getClass.getResourceAsStream("/XML/Etc.nx.gz"))(stream => {
      bytes = GzipParser.readGzip(stream)
    })

Really? You're loading compressed NX files?
I find it hilarious how people are using the NX file format.
NX was designed first and foremost for me, for my C++/Rust, so that I can memory map the file and access the data directory with almost no loading and without using up significant amounts of memory.
Meanwhile here you are compressing your NX files so that not only do you have all the loaded NX data in memory, but you also have the decompressed file itself in memory.
The set of NX files for servers (so no bitmaps or sounds) was 451 MB back in GMS v141. Do you really want to throw that much memory away, plus the CPU time to decompress that, just for the sake of saving a bit of hard drive space?

I did this because most files will be parsed (almost) completely and I will only save the information I need in the memory.
So the extra allocated memory for the files will be cleared right after I have collected all data.

As for the compression, I do not understand the problem of it taking a little more cpu time, as it will only be called when the server is started.

[retep998]

I did this because most files will be parsed (almost) completely and I will only save the information I need in the memory.
So the extra allocated memory for the files will be cleared right after I have collected all data.

As for the compression, I do not understand the problem of it taking a little more cpu time, as it will only be called when the server is started.

I just find it incredulous that anyone would be willing to have their server take longer to startup just to save a bit of hard drive space.

[Hilia]

You keep writing unnecessary things like keywords for constructor parameters.

class LazyNXFile(val buffer: ByteBuffer)

can just be

class LazyNXFile(buffer: ByteBuffer)

Aren't the unnecessary keywords just a preference of the coder?,Like in the Microsoft C# Roslyn compiler they use a lot of the unnecessary "this" 's even though Microsoft's code analyses says:"Nono you can remove that". Where as the coders say: "It is just what you prefer". Doesn't the same count for the example you shown? (Unless there is a performance impact)

EDIT:
(Example where they don't use "this" it:https://github.com/dotnet/roslyn/blo...mblyVersion.cs
Example where they use "this" https://github.com/dotnet/roslyn/blo...MemoryBlock.cs)

[Minike]

I just find it incredulous that anyone would be willing to have their server take longer to startup just to save a bit of hard drive space.

I can totally understand your argument, but I saw no need for a few seconds of increased performance in a process that should only happen maybe once a week.
But it is not like it saves much disk space and neither takes up much space in total, so this is totally arguable.

[Minike]

A few updates;

• Removed PKGNX, added custom PKG4 reader
• Replaced most 'null' values with Options
• Equips now save and display

So basically the login server is almost finished except for the PIC

Spoiler:

I also have added a database script for creation with a default account 'Admin' with password 'Test123'

[aaronweiss]

Aren't the unnecessary keywords just a preference of the coder?,Like in the Microsoft C# Roslyn compiler they use a lot of the unnecessary "this" 's even though Microsoft's code analyses says:"Nono you can remove that". Where as the coders say: "It is just what you prefer". Doesn't the same count for the example you shown? (Unless there is a performance impact)

The difference is that there's an actual idiom here. Having it actually explicitly declare val means that you'll have to stop to read it (the alternative option is var) instead of correctly assuming that it's immutable. It's also to match pattern syntax and construction syntax better.

- - - Updated - - -

A few updates;

• Removed PKGNX, added custom PKG4 reader
• Replaced most 'null' values with Options
• Equips now save and display

Clearly, you took the criticism to heart. Your NX reader is still basically Java (for loops, and Iterable). You replaced things with Option, but you don't match on the options! It's still better because at least the ability to have no value now exists explicitly in the type system, and not just implicitly everywhere. You should seriously look into Seq and map, fold, et. al.

[Minike]

Clearly, you took the criticism to heart. Your NX reader is still basically Java (for loops, and Iterable). You replaced things with Option, but you don't match on the options! It's still better because at least the ability to have no value now exists explicitly in the type system, and not just implicitly everywhere. You should seriously look into Seq and map, fold, et. al.

I am not quite sure if this also applies for Scala, but it does for C#; using things such as '.foreach' instead of the default 'for loop' creates functions overhead, so I have the tendency to avoid them.

As for the Options, I prefer using the 'for' syntax for them as you could see them as an Iterable with either one or zero values and when requiring a default value 'getOrDefault' suffices.

[aaronweiss]

I am not quite sure if this also applies for Scala, but it does for C#; using things such as '.foreach' instead of the default 'for loop' creates functions overhead, so I have the tendency to avoid them.

Oh, man. "Functions overhead." This is rich. Thanks for the laugh, bud.

You're writing a functional language. You should use functional style. You're not going to notice any performance differences, and your code will be far easier to read. There isn't much of a reason to use Scala at all if you're just going to write it as if it's Java or C#.

As for the Options, I prefer using the 'for' syntax for them as you could see them as an Iterable with either one or zero values and when requiring a default value 'getOrDefault' suffices.

I meant there's cases where you have if-else statements with isEmpty being the condition. In those instances, you should be using a match. It's far cleaner. Also, please go read the docs for Seq already and stop talking about or using Iterable in Scala. I've mentioned it several times now.

[Minike]

You're not going to notice any performance differences, and your code will be far easier to read.

Actually, unless some compiler magic is going on, which I doubt as I personally think it will behave somewhat like the C# equivalent (LINQ), thus causing minor performance loss, or large losses if you are using it incorrectly (e.g. unknowingly causing to many iterations)

I meant there's cases where you have if-else statements with isEmpty being the condition. In those instances, you should be using a match. It's far cleaner.

I dont think many people would use a 'switch case' statement in Java or C# when there are only 2 options (or actually, 1 option and a default). So I neither see they gain of matching everything, everywhere.
Though I do have a few cases where it might add to the clarity of the code, so I probably use a match there instead.

Also, please go read the docs for Seq already and stop talking about or using Iterable in Scala. I've mentioned it several times now.

Here is the reason why I talk about Iterable, rather than Seq

Spoiler:

[aaronweiss]

Actually, unless some compiler magic is going on, which I doubt as I personally think it will behave somewhat like the C# equivalent (LINQ), thus causing minor performance loss, or large losses if you are using it incorrectly (e.g. unknowingly causing to many iterations)

It won't. It's inconsequential.

I dont think many people would use a 'switch case' statement in Java or C# when there are only 2 options (or actually, 1 option and a default). So I neither see they gain of matching everything, everywhere.

None is not a default, and no, obviously not because switch statements only work on numbers and strings in Java. Match statements are more flexible and are used to destructure patterns. Match statements are the idiomatic way to do these things in Scala.

Though I do have a few cases where it might add to the clarity of the code, so I probably use a match there instead.

No kidding. It's not like I said that already.

Here is the reason why I talk about Iterable, rather than Seq

Spoiler:

You're unnecessarily over-abstracting (as most lousy Java programmers do). Seq is for collections with a sequential ordering. NX files have a sequential ordering. At the very least, start taking advantage of the fact that Scala's Iterable provides functional goodies, too.

[retep998]

Actually, unless some compiler magic is going on, which I doubt as I personally think it will behave somewhat like the C# equivalent (LINQ), thus causing minor performance loss, or large losses if you are using it incorrectly (e.g. unknowingly causing to many iterations)

This is called premature-optimization. Please avoid it.
Never make claims about the performance of something until you've benchmarked it.

[Minike]

This is called premature-optimization. Please avoid it.
Never make claims about the performance of something until you've benchmarked it.

I am well aware of this, as you basically end up 'reinventing the wheel and make it square'
The thing I want to prevent though, and trust me I have seen this happening a lot of times in other people their code, is to create one humongous psuedo-query just because it looks 'fancy'.
This while a mere combination of a filter and foreach could have the same result.
So while being 'fancy' you might have accidentally looped thrice over twice the amount of data. (please save me the part of 'you should know what each function does')

So personally I think things such as 'foreach' is just abusing the functional syntax as it hurts the clarity of the code.