u3d-community / U3D

Open-source, cross-platform 2D and 3D game engine built in C++
https://u3d.io
MIT License
187 stars 30 forks source link

Add fmt for string formatting #18

Closed SirNate0 closed 1 year ago

SirNate0 commented 1 year ago

Implementation of request #9. Adds the fmt library for a FormatString utility with {}-style format to eventually replace the printf style formatting with %s, %d, %f, etc used by ToString("format-str",...). Further, adds U3D_LOG____F macros that use this fmt-style format string, while the URHO3D_LOG____F macros use the printf-style through a deprecated call to Log::WriteFormat (use Log::WriteFormatted for the {}-style).


Before this is merged, I would like to:

SirNate0 commented 1 year ago

The web build fails, as the tools are built with an incomplete "Mini" version of Urho, which I did not set up to include fmt. I will go ahead and add it as a dependency to them if there are no objections.

eugeneko commented 1 year ago

By the way, you can avoid adding new macros for logging. We rewired LOG macros to format with {} syntax by default, and LOGF with % syntax, both are using fmt of course

SirNate0 commented 1 year ago

That is an excellent idea. I will switch to that instead.

With a template specialization or overload, it should be possible to still call a completely unformatted version of the logging code, so no extra runtime cost as well.

SirNate0 commented 1 year ago

Implemented Eugene's suggestion. Also changed to fmt 9.1, and since those were fairly comprehensive changes, I just started from a clean slate so the history is cleaner.

To upgrade the log macros, I used the following script. It may be useful to some users, so I'm putting it here, or we could add it under script/utils.

#!/usr/bin/env python3

import re
import os
import sys
import re
from glob import glob

print(sys.argv)

bad = []

def convert(f):
    if f.name.endswith('Log.h'):
        return None
    content = f.read()
    # URHO3D_LOGF is only used in the definition, so no need to specially handle it with a non-string first argument
    for level in ['TRACE','DEBUG','INFO','WARNING','ERROR','RAW']:
        matches = list(re.finditer(f'URHO3D_LOG{level}F'+r'\(((\s*"([^"]|\\")+")+)'
                        ,content))

        calls = list(re.findall(f'URHO3D_LOG{level}F',content))
        if len(matches):
            print(f.name)
        if len(matches) != len(calls):
            print('ERROR, could not convert all of the calls in ',f.name)
            bad.append(f.name)
        for m in matches:
            print(m)
            print(m.group())
            #print(content[m.start():m.end()])
            newstr = m.group().replace('{','{{').replace('}','}}')
            for c in 'diulfcsxp':
                preserve = 'cx' # Assume remaining are handled correctly by default: (unsigned) integers/longs, floats, strings, and pointers
                if c not in preserve:
                    newstr = re.sub(r'([^%])%'+c,r'\1{}',newstr)
                else:
                    newstr = re.sub(r'([^%])%'+c,r'\1{:%c}'%c,newstr)
            newstr.replace('%%','%')

            #newstr = re.sub(f'^URHO3D_LOG{level}F',f'U3D_LOG{level}F',newstr)
            newstr = re.sub(f'^URHO3D_LOG{level}F',f'URHO3D_LOG{level}',newstr)
            print(newstr)
            content = content.replace(m.group(),newstr)
            assert(m.group() not in content)
    return content

files = []
[files.extend(glob(f'../../Source/{d}/**/*.{ext}',recursive=True)) for ext in ['h','cpp'] for d in ['Samples','Urho3D','Tools']]
for fname in files:
    #print(fname)
    with open(fname,'r') as f: # '../../Source/Urho3D/Graphics/OpenGL/OGLGraphics.cpp'
        c = convert(f)
    if c is not None:
        with open(fname,'w') as f: # '../../Source/Urho3D/Graphics/OpenGL/OGLGraphics.cpp'
            f.write(c)

print('\n\nFINISHED\n\n')
if bad:
    print('Failed on \n' + '\n'.join(bad))
SirNate0 commented 1 year ago

To fix the linker errors in the shared builds, I am going to switch to FMT_HEADER_ONLY.

SirNate0 commented 1 year ago

@eugeneko I saw that RBFX is using CMake's target_compile_definitions to specify FMT_SHARED/FMT_EXPORT in building fmt. Is there a reason for taking this approach over always using the HEADER_ONLY form?

eugeneko commented 1 year ago

Slightly faster compilation, probably? I don't think it matters much.