Switch to C++17 and use string_view

[Assumeru]

As mentioned in #251.

This PR attempts to reduce the number of superfluous string allocations in MyGUI. Mostly by employing string_view almost everywhere, but also by fixing a few places that copied a (return) value instead of taking it by reference, and (in places I spotted the issue) by preventing redundant UString->string->UString conversion chains.

Some aspects of the API where left as const std::string&, mostly the ones that ended up calling Ogre functions taking arguments of that type. It might be worth changing more of those over to string_view in the future though such a change would necessarily favour the other rendering targets over Ogre in terms of performance -- minor though the difference might be.

This is a pretty big change with regard to backwards compatibility. Upgrading shouldn't be particularly hard, it wasn't for OpenMW.

I've tested compilation on Windows 10 / MSVC and Ubuntu 22 / Clang 14.

[Assumeru]

@psi29a maybe you can verify this compiles on Mac?

[Altren]

I can check that

[psi29a]

We an also setup github actions to do that for us. :)
We can liberate what we've done over at openmw.

[Altren]

That would be great, since travis-ci is broken now

[Altren]

@Assumeru there are few review comments above before we can merge this

[Altren]

And thank you for you work, glad to see so big and good PR

[Assumeru]

@Assumeru there are few review comments above before we can merge this

The thing about cmake policies AnyOldName3 mentioned or the commits you added?

And thank you for you work, glad to see so big and good PR

Thanks for even being willing to entertain the notion of a 2k line PR 😅 Once this is merged I'll probably end up submitting some more to get rid of a few compilation warnings (implicit casts, c-style casts) and maybe to employ unique_ptr so we can eliminate a few destructors.

[Altren]

No, there are my review comments above. The only critical one is about eventClipboardChanged

[Altren]

Once this is merged I'll probably end up submitting some more to get rid of a few compilation warnings (implicit casts, c-style casts)

I'm waiting for this PR before pushing my massive changes, so don't rush into doing that.
Also I somewhat prefer c-style casts for basic types, since otherwise code becomes less clean. So don't change that unless really necessary

[Assumeru]

No, there are my review comments above. The only critical one is about eventClipboardChanged

I'm not seeing any other comments. Did you leave them as a draft?

[Altren]

No, there are my review comments above. The only critical one is about eventClipboardChanged

I'm not seeing any other comments. Did you leave them as a draft?

@Assumeru my bad. sorry

[AnyOldName3]

I don't have the button to unresolve #252 (comment) so I'm making another top-level comment so my last post has some visibility. I don't think the thread should have been resolved.

[Altren]

I now hope your planned changes involve removing the const_casts from MyGUI_Delegate.h somehow 😛

@Assumeru Done

[Assumeru]

I now hope your planned changes involve removing the const_casts from MyGUI_Delegate.h somehow 😛

@Assumeru Done

Awesome.

[akortunov]

A virtual means that the client applications may derive from class and override a virtual method. When you change the signature of virtual method, client applications which override it will stop to compile.

So when you modify a library's public API, you usually keep both old and new API, but mark old API as deprecated. Later you remove deprecated API as a part of cleanup for major release and post a migration guide. In this case developers of client applications have enough time to migrate their code to upstream API. For example, API from Qt4 was deprecated in Qt5 and removed only in Qt6.

So far MyGUI 3.4.3 is incompatible with MyGUI 3.4.2 and earlier versions, but release changelog does not directly contain any info about it.