4
\$\begingroup\$

I'm playing around with making random items and decided to implement the following structure.

#include <stdlib.h> //uints 
#include <limits>
#include <random>
#include <iostream>

class RandomItem {
private:
    struct _physical_properties { uint16_t length; uint16_t width; uint16_t sqew; uint16_t weight; };
    struct _color { uint8_t r; uint8_t g; uint8_t b; uint8_t a;};
    //etc
public:
    _physical_properties physical;
    _color main_color;
    _color secondary_color;
    uint16_t durability;
    uint32_t type;

    RandomItem() = default;
    RandomItem(uint32_t _type) {
        union thing {
            RandomItem item;
            uint8_t buf[sizeof(RandomItem) / sizeof(uint8_t) ];
        };
        thing bad{};
        std::random_device dev;
        std::mt19937 gen(dev());
        std::uniform_int_distribution<> dist(0, std::numeric_limits<uint8_t>::max()); 
        for (auto& b : bad.buf) {
            b = static_cast<uint8_t>(dist(gen));
        }
        *this = bad.item;
        this->type = _type;
    }
};

int main() {
    RandomItem a(1);
    RandomItem b(2);
    std::cout << "item type: " << a.type << " Some values: " << (int)a.physical.length << ", " << a.durability << std::endl;
    std::cout << "item type: " << b.type << " Some values: " << (int)b.physical.length << ", " << b.durability << std::endl;
    return 0;
}

(edited slightly to post here in a more minimal form...using iostream instead of fmt sticking in one file etc)

seeing as how:

  1. The total number of properties in the class can increase and have (potentially) different types
  2. I don't particularly feel like
prop1 = dist(gen);
prop2 = dist(gen);
prop3 = dist(gen);
//etc

is very scalable...

I wrote the displayed constructor and that's specifically what I'm asking for feedback on (rest of the dummy class is just for the example)

It uses a union between the item and a buffer and fills that buffer with random values then sets the current object equal to the unionized item.

This feels hacky however might be better than alternatives (individually setting each variable manually or the other end of the gross hack scale making a macro that expands to setting each variable)

Are there any major gotchas with doing this?

Am I relying on undefined behavior and or will this style of code introduce bugs down the line? (as far as I can see it doesn't quite matter what the endianness of the computer is 'cause the value is random)

Is this the best way to implement my goal? (set all the variables and nested variables to random values on construction).

I'm posting here instead of StackOverflow because it appears to work (compiles and and I assume any answers would just be opinion-based. I'm not working with other people and for now, I'm just experimenting with things. (this isn't being used in a project but I might add it into one in the future.).

Is it "better" to make the class nothing but POD without a constructor and then implement a function that does pretty much the same code and return the constructed item? (something like)

Item& randomItem(uint32_t type){
    union thing{
        Item item;
    //snip 
    ...
    return item;
} 

To avoid the hacky *this = item; code (smell?)

Of course, if I was actually writing a game or similar program that implemented a class of random variables I'd probably have some constraints for what the random values for different types of items could be. (For example, length/width might be a distribution between (300-1000) instead of (0-maxValueOfType) so maybe this is just a contrived example. However, if anyone has any feedback on the rest of the concept I'd be interested.

\$\endgroup\$
  • \$\begingroup\$ Filling a byte array with random values and type-punning it as an Item is definitely undefined behavior ... \$\endgroup\$ – L. F. Apr 21 at 9:04
  • \$\begingroup\$ @L.F. I think it's fine since it's a trivial type. Am I mistaken? \$\endgroup\$ – sudo rm -rf slash Apr 28 at 3:27
  • \$\begingroup\$ I mean this link suggests it could raise a sigbus in c++ so what I wrote is valid c but UB in c++...I went down a rabbit hole of stack overflow questions wondering if I memcpy'ed a buffer into the object instead of used a union or reinterpret cast'ed it but both options also seem to be able to trigger UB so I gave up \$\endgroup\$ – BlueLightning42 Apr 28 at 15:57
  • \$\begingroup\$ This might be less undefined behaviour if you use std::launder() on the randomized object you created. \$\endgroup\$ – G. Sliepen Sep 25 at 9:00
1
\$\begingroup\$

In theory, I think this is allowed because your Items are trivial types, but this is pretty ugly.

I think you can do better using std::array.

struct _color { uint8_t r; uint8_t g; uint8_t b; uint8_t a;};

could be

struct _color {
    auto& r() { return data[0]; }
    // etc
    void randomize() { /* fill data with random stuff */ }
    private:
        std::array<uint8_t, 4> data;
};

P.S. I think you will find that uniformly randomly setting r, g, b, and a doesn't look random to humans.


P.P.S. There is a proposal to add something like this:

_color color;
for... (auto& component : std::meta::members(color)) {
    // do stuff with component
}

I haven't followed this proposal but it sounds like it would help here.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ IIRC, trivial types are only allowed to be reinterpreted from a byte array if the array is memcpy’ed from another object of the same type. So this is actually UB. For nontrivial types, even this is not allowed. \$\endgroup\$ – L. F. Apr 28 at 8:08
  • \$\begingroup\$ I think you are always allowed to read your own byte array with additional guarantees for trivial types: en.cppreference.com/w/cpp/language/…. Since each byte must be aligned, I think writing to the bytes is ok since at worse you'll overwrite padding. \$\endgroup\$ – sudo rm -rf slash Apr 28 at 13:51
  • \$\begingroup\$ The random bytes may end up with trap representations; reading therefrom is UB. Regardless, I was wrong in my previous comment - even reading from memcpy’ed bytes is UB, since the pointers fail to point to objects of the appropriate type. This will change in C++20, with the adoption of implicit creation of objects — see eel.is/c++draft/intro.object#10. Again, even then reading from random values is probably UB. I guess we need a language lawyer question on Stack Overflow to discuss in detail. \$\endgroup\$ – L. F. Apr 28 at 14:24
  • \$\begingroup\$ Let me know somehow if you post a question :) \$\endgroup\$ – sudo rm -rf slash Apr 28 at 14:51
  • \$\begingroup\$ this is a valid way of storing several values that have the same type...but in my example I purposefully used uints instead of int char etc to illustrate some of the types being uint8_t some uint16_t some uint32_t...this assumes they are all uint8_t big \$\endgroup\$ – BlueLightning42 Apr 28 at 16:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.