TDialogRes overhaul

Great idea! This class seems to be an incomplete implementation leftover from BC5.02/OWL5 days and not touched since.

A series of posts about dialog templates from Raymon Chen's blog:

The evolution of dialog templates – Introduction

The evolution of dialog templates – 16-bit Classic Templates

The evolution of dialog templates – 32-bit Classic Templates

The evolution of dialog templates – 16-bit Extended Templates

The evolution of dialog templates – 32-bit Extended Templates

The evolution of dialog templates – Summary

Hi @jogybl,

Curious to check out your latest changes to modernise the OWLNext code base, I did a build test of the trunk, and while clicking through on some new warnings, I stumbled upon an issue with TDialogRes::GetText.

After your recent refactoring [r8657], the first part of the function no longer does anything useful, and your new code uses _tcsncpy, which (among other issues) may truncate without null-termination.

Tip: Never use strncpy. The C++ standard library now has safer alternatives. If you want to allow truncation, use std::strncpy_s, or if not, use std::strcpy_s (and explicit truncation error handling).

Possible fix, allowing truncation:

//
/// Retrieves the string selected by \p which.
///
/// \returns The size of the string written to the buffer, excluding the null-terminator (which is
/// always written).
///
/// \note The string is truncated if too large to fit in the given buffer. However, the buffer is
/// still null-terminated.
//
auto TDialogRes::GetText(LPTSTR buf, int bufSize, TDlgResText which) const -> int
{
  PRECONDITION(buf && bufSize > 0);
  const auto empty = tstring{};
  const auto* const s =
    (which == drtMenuName) ? &DialogData.MenuName :
    (which == drtClassName) ? &DialogData.ClassName :
    (which == drtCaption) ? &DialogData.Caption :
    ∅
  const auto n = static_cast<size_t>(bufSize - 1); // Max string size.
  [[maybe_unused]] const auto r = _tcsncpy_s(buf, bufSize, s->c_str(), n); CHECK(r == 0);
  return static_cast<int>(min(n, s->size()));
}

@jogybl wrote:

This function is now totally redundant and is marked as deprecated. I am going to simply remove it in the near future

Makes sense. Still, I learned something today (after hours discussing C++ standard behaviour with Copilot): Don't use a reference declaration and ternary expression together for selective binding. It is too easy to have an unintentional temporary generated. For such selective binding, use an explicit pointer instead.

https://gcc.godbolt.org/z/5sx9K9qrv

With the changes in [r8737] the overhaul of the TDialogRes class is done for now.
The class correctly handles both classic and extended templates and also retrieves the list of controls in the templates, as demonstrated in the new TDialogResTest example in the Classes project.

In the future, if requested, functions for manipulating the templates can be added.

After the overhaul to implement support for extended dialog templates [bugs:#593], here is a patch with some further clean-up you might consider:

• CHG: TDialogRes is now final.
• CHG: TDialogRes::Resource is now a TResource instance, not a pointer to an instance.
• CHG: TDialogRes destructor has been eliminated.
• CHG: TDialogRes::IsOK is deprecated, and now always returns true.
• CHG: TDialogRes::GetSize is now inline and returns int, not DWORD.
• CHG: TDialogRes implementation now uses TResource::Get rather than operator calls.
• CHG: Based the enum for RtDialog on int, eliminating casts.
• BUG: TDialogRes::GetTemplate and GetTemplateEx are not const-correct.
• BUG: TDialogRes::IsDialogEx may dereference invalid template pointer. Resolution: A check on the buffer size is now performed before dereference.