2
\$\begingroup\$

I currently am working on a game engine written in C++ with libuv backed for io system calls. I have a FileSystem class that just 'works'. I am allow to read files without blocking but the current design I have is lacking a bit. I am hoping some of you all could comment on my code and help me improve it.

Here are some noticeable changes:

  1. To start off FileContext->data[64]. How would I get rid of the fixed size? Basically I saw examples (there really isnt much examples of libuv) that show them using a char* instead of the fixed size. I tried using a char* but I got a permission denied error.
  2. This one is based on the first one. I have FileContext->content and FileContext->promise which in theory would return the same value after some time. Basically the former get written to by libuv using the char [64]. I do this because other wise I would not have the full file content string. The latter get it value set as the former value once the task get completed. How would I get rid of one or the other? I rather not have both. Even better would be on how to get rid of both of them and just have FileSystem->read return the promise object instead.
  3. FileSystem->_contexts is a raw pointer. I made it a std::unique_ptr and I ran into a odd bug. Basically my program crashed with undefined behavior and I wasn't quite sure so I made it a raw pointer again and it started working again.
  4. Should FileSystem->read return a std::promise or a std::future object?
  5. How would I go about dealing with errors? For example a permission denied for read a file error. I want to use std::exceptions but I am not quite sure and would like comments on that as well.

I think that is all that I was able to notice on my design, if you see something feel free to comment on that as well as what is above. I would very much appreciate it, thank you.

#ifndef RISKY_ENGINE_INSTANCES_FILE_SYSTEM_HEADER
#define RISKY_ENGINE_INSTANCES_FILE_SYSTEM_HEADER

#include <future>
#include <memory>
#include <string>

#include "risky/engine/prefix.hpp"
#include "risky/engine/core/ClassMaker.hpp"
#include "risky/engine/instances/Instance.hpp"

#if defined(RISKY_USE_LIBUV)
    #include <uv.h>
#endif

namespace risky {
    namespace engine {
        class Engine;
        class FileSystem;

        struct FileContext {
            std::string content;
            std::shared_ptr<Engine> engine;
            std::promise<std::string> promise;
            std::shared_ptr<FileSystem> fileSystem;
        #if defined (RISKY_USE_LIBUV)
        //  char* data;
            char data[64];
            uv_fs_t open;
            uv_fs_t read;
            uv_fs_t close;
            uv_buf_t buffer;
            uv_loop_t* loop;
        #endif
        };

        class FileSystem: public Instance {
        public:
            explicit FileSystem(const std::shared_ptr<Engine>& engine) noexcept;
            virtual ~FileSystem() noexcept;

            virtual std::promise<std::string>& read(const std::string& file) const noexcept;

        protected:
            std::vector<FileContext*> _contexts; //Todo: make these unique pointers?

        #if defined(RISKY_USE_LIBUV)
            uv_loop_t* _loop;
        #endif  
        };

        DECLARE_CLASS(FileSystem);
    }
}

#endif
#include <iostream>

#include "risky/engine/core/Engine.hpp"
#include "risky/engine/instances/LogService.hpp"
#include "risky/engine/instances/FileSystem.hpp"

namespace risky {
    namespace engine {
        DEFINE_CLASS(FileSystem, false, true, Instance) {

        }

        #if defined(RISKY_USE_LIBUV)
        static void on_read(uv_fs_t* request) noexcept {
            FileContext* context = static_cast<FileContext*>(request->data);
            uv_fs_req_cleanup(request);

            if (request->result < 0) { //Todo: throw exception?
                fprintf(stderr, "error: %s\n", uv_strerror(request->result)); 
                return; 
            }

            if (request->result != 0) {
                context->content += context->data;
                std::cout << context->data << std::endl;
            //b std::cout << "File content: " << context->content << std::endl;
                uv_fs_read(uv_default_loop(), &context->read, context->open.result, &context->buffer, 1, -1, on_read);
            } else {
                context->promise.set_value(context->content);
                uv_fs_close(request->loop, &context->close, context->open.result, NULL);
                //Remove from vector

                delete context;
            }
        }

        static void on_open(uv_fs_t* request) noexcept {
            FileContext* context = static_cast<FileContext*>(request->data);

            if (request->result < 0) {
                fprintf(stderr, "error: %s\n", uv_strerror(request->result)); //Todo: throw exceptions?
                return;
            }

            context->buffer = uv_buf_init(context->data, sizeof(context->data));
            uv_fs_read(request->loop, &context->read, request->result, &context->buffer, 1, -1, on_read);
            uv_fs_req_cleanup(request);
        }

        #endif

        FileSystem::FileSystem(const std::shared_ptr<Engine>& engine) noexcept : Instance(engine), _loop(engine->getEventLoop()) {

        }

        FileSystem::~FileSystem() noexcept {

        }

        std::promise<std::string>& FileSystem::read(const std::string& file) const noexcept {
            FileContext* context = new FileContext(); //Todo: make this a uqiue pointer?
        //  std::unique_ptr<FileContext> context = std::make_unique<FileContext>();

            #if defined(RISKY_USE_LIBUV)
                context->loop = _loop;
                context->engine = _engine;
                context->open.data = context;
                context->close.data = context;
                context->read.data = context;
            //  context->fileSystem = shared_from_this();

                uv_fs_open(context->loop, &context->open, file.c_str(), 0, 0, on_open);
            #endif      

        //  _contexts.push_back(context);
            return context->promise;
        }
    }
}
```
\$\endgroup\$

Your Answer

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

Browse other questions tagged or ask your own question.