zturtleman / mm3d

Maverick Model 3D is a 3D model editor and animator for games.
https://clover.moe/mm3d
GNU General Public License v2.0
115 stars 23 forks source link

Discussion: typedef const char *utf8? #71

Closed m-7761 closed 5 years ago

m-7761 commented 5 years ago

I don't normally do this, but in writing the UI parts I started to use utf8 as a shorthand for "const char *" because that is sort of long. I found myself writing it a lot.

I'm asking, because it's not inconvenient, given how often the type is used. So I'm curious if this can be defined somewhere, and used elsewhere, in the core code, as a convention.

If so, there was an mm3dtypes.h file, but I removed it the other day, because it was a one-liner that said typedef float float32_t; so I thought it overkill.

I think a types header can be a useful place to predeclare pretty much every type in the library. It can be a useful place to see all of the types in one place. And it's a natural place to start a precompiled header. By putting the predeclarations in one place, it's unnecessary to put predeclarations all over the place.

But I'm just asking about utf8, since right now I'm trying to avoid using it outside of UI definition code. I.e. code that would otherwise pull in Qt headers.

m-7761 commented 5 years ago

To be honest, I think it's a good idea to establish a top-level header. Since std containers are used liberally, I would rather it include all of the std headers at the top.

And another eccentric quality of the code, is the files tend to #include private headers before public headers. That's just kind of a really bizarre habit that I've never seen anywhere. It's really not good since it risks something in a private header changing the behavior of a system header. So, I think it would be better to put all of the system headers in a top-level header, since it would be less work (reversing the order) that way. It would be good to see all of the system headers in one place. I mean, at least for the core code.

P.S. I've been removing stuff from the mm3dport.h header where possible. And adding some things. I think there is no reason the code can't be built with Visual Studio. There's very little non-portable pieces, so it's not difficult to remove them. To be honest, I've never developed with Linux tools that wasn't a frustrating slog compared to working with VS. I think it's 20 times as productive. So I recommend all code work first with VS, and try to do as much development with it as possible.

zturtleman commented 5 years ago

I'm not particularly a fan of hiding that it's a const pointer. If utf8 was a class it would probably be passed as a const reference. The proposal is setTitle( utf8 title ) code that is inconsistent with all other multi-value (array, std::string) type usage.

Yeah, header order is odd. Though also need to check if there are intentional defines to alter system headers.

I don't plan to test or actively maintain Visual Studio support but making changes to build using VS is fine as long as mingw still works.

m-7761 commented 5 years ago

My view is it's naive to pass around things like std::string and using C-string is generally best. Though for modules, I mean, my UI library has a wrapper that is called c_string that accepts any kind of char container while weeding out things that look like nullptr. That's a taller order however.

The COLLADA-DOM library I've developed has a very good string_view like class, for doing string comparisons. But it's for XML where string comparisons are commonplace. In this case, I think it would be overkill.

Anyway, I don't believe it's hiding. It's shortening code to make it more legible so things don't run long horizontally, and it also communicates utf8. Everyone knows what utf8 is. I think it's not a bad name. The word utf8 doesn't strike me as a class name at all. Plus the library uses capitalization for classes broadly.

I definitely need a centralized header. Presently I'm using a forced-include mechanism to do my port work. Yesterday I built and linked the ported code for the first time, now heading into running/debugging it. The Win32 parts seemed like maybe they are not actively maintained. I don't know. Maybe the GCC for Windows system uses Win32 APIs. I've built it in Visual Studio.

zturtleman commented 5 years ago

Yes, Maverick built with mingw uses the Win32 API. All the Win32 code was reviewed and a lot of file access was changed from C standard library to Win32 API to support non-ASCII paths for the Maverick 1.3.10 release in 2018.