Skip to content
This repository has been archived by the owner on Aug 1, 2022. It is now read-only.

Massive angelscript gameworld and game driver updates. [Main] #115

Open
wants to merge 94 commits into
base: main
Choose a base branch
from

Conversation

PyroFire232
Copy link

@PyroFire232 PyroFire232 commented May 9, 2021

Massive gameworld and engine update.

I done did a programming.

I broke your map generator thing, sorry.
I'm probably going to make you a better one anyway.

I broke your inventory too, but I can do a better one of that too.

Changes

  • New Object-Oriented systems and resource management overall, and engine stuffs.
      • Precise tickrate and framerate handler (I fixed your timers, basically).
      • Internal tickrate, framerate, and .tick tracking.
      • .ogg playback libraries with a working example (early version).
      • Entity.SetMaterial(); and CMaterial library to create materials.
      • Renamed the update functions to update(tick, interp), updateAlways(tick, interp), and render(interp), and renderAlways(interp).
      • Renamed "World::" to "Environment::" (in angelscript only) due to naming conflicts.
      • Renamed "Model::" to "CModel::" (in angelscript only) due to naming conflicts.
      • Added filepaths to top of root script files so assets can be moved out of the SCPCB folder later.
      • Generalized assets loading system, making the game load much faster for quicker code iterations.
      • Added *.INI script definitions (unused and untested).
      • Player is no longer an itty bitty midget.
  • CBR mesh format implemented with 60 rooms (they take forever to load, so it only loads the first 4).
      • Working collision & textures.
      • Sorted and verified a bunch of models
      • Error-corrected some missing textures for the rooms
  • Entirely revamped GUI system with parenting, inheritance and alignment, and re-created all the essential GUI elements in the new format, including:
      • Loading screen, HUD, Main Menu, Pause menu
      • Debug menu in pause menu with options for noclip, teleport to 0,0, spawning rooms, and a few other bits and pieces.
  • Entirely new items, rooms and objects systems.
      • Self-driving and modular, requiring zero setup code with relatively clean namespaces, with working examples of inherited behavior.
      • Functioning door entities, animations and buttons, samples of which are set to spawn precision-aligned with each room (coordinates for reference), all keycards and a few other items, item skins, and more.
      • All rooms appear to be 200x200 game-units in dimensions, with a 1 unit buffer gap. the front door is aligned precisely at -100 -1 from origin.
  • I probably forgot something.

PyroFire232 added 11 commits May 9, 2021 09:03
Intended to make it much easier to create item definitions and manage individual item behavior.
Also added some general World classes like model/picker manager and icon manager
Useful references for item definitions and also draw code + some behavior.
Added options for .skin, .picksound, and .scale. automated model scaling on instantiation.
Primary GUI engine completed, and most of the base panels are working reasonably well. Menu states in place. All sorts of stuff. Cleaning up Main.as considerably. Console not done yet. Icon editor not done yet. Easy additions though. Pushing early for show and integration prep, like adding Vector*Vector.
And fixed loadscreen + a few tidy-ups



/* Originals --------------------------------------
Copy link
Collaborator

@juanjp600 juanjp600 May 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Including a copy of the original source code here is just noise. If we have to look at it as a reference, we can just go to the original repository or the OldSrc directory.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It helps with organization for me at least.

Copy link
Author

@PyroFire232 PyroFire232 May 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will remain attached to the item until it is imported/fully refactored. It contains essential references conveniently next to the definition, such as strings to be localized, sounds, and heal functions. The issue is many components of the items are scattered, i.e I do not see the sounds in there, hence moving all the code related to that particular item into that item, to help find it. This is actually stored here for my refactoring reference purposes, and although I could move it, I do not see much benefit as it helps to sort out completed or near-completed items from unstarted ones. I also note that the other items in OldSrc do not contain much of, if any of this information, also with a lot of items missing. I have moved the other to-be-done items into a folder in the OldSrc folder, and can manage my references there, where they do not apply to a particular data file. Originals may be removed when we are satisfied the item contains all the information desired from the original reference material.

textPanel.height=8;
textPanel.margin={1,0,1,0.5};

@ConsoleMenu_ButtonPanel=menu_Console_ButtonPanel(@canvasMain);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using namespaces is the appropriate solution over using snake case here

Copy link
Author

@PyroFire232 PyroFire232 May 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, I'll go ahead with this cleanup.
I'll leave GUI elements as they are, as this can introduce naming conflicts in the GUI namespace that I don't want to have to deal with, and anything other than GUI::[x] or GUI[x] just doesn't feel right. Just me?
Actually, having a look at the GUI namespace, the elements can be added to GUI:: without issue.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pausemenu has been refactored into the final format. the other menus will follow suit

@Saalvage
Copy link
Member

General word of advice: It's better to have multiple smaller pull requests than one giant one. E.g. the item and GUI system should definitely be two seperate pull requests. This makes it much easier to review them and eventually merge.

slightly broke text entry, but added hook for console menu.
Console menu is not plugged in yet
I managed to make it even better.
Problems getting Timer:: to work, so it's in the tick hook for now.
void update(float timeStep);
void drawGame(float interpolation);
void drawMenu(float interpolation);
void updateTick(uint32_t tick,float interp);
Copy link
Collaborator

@juanjp600 juanjp600 May 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The signature of this method is ill-defined. The tick callback does not work with interpolation values, it works with timestep lengths.

Copy link
Author

@PyroFire232 PyroFire232 May 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting problem that I do not have a clean answer for.
Namely, how to manage animation frame advancement at the tick/framerate with regards to pausing, and frame interpolation.
This is a part of a bigger discussion that is worth having regarding how the timing systems are intended to work and what they need to do, along with syncronizing tick advancement with frame advancement.

int tickRate;
int tickStep;
float sinceLastTick;
float avgTickRate;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tick rate is supposed to be fixed. What purpose does this serve?

Copy link
Author

@PyroFire232 PyroFire232 May 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because there is no guarantee of how long a tick will or is taking, against whether or not the program has slowed down for outside reasons, or even when the timer functions will be called. This calculates what the real tick rate is, and tries to adhere to the goal tick rate as closely and smoothly as possible. Attempts can be made to compensate for this, but I cannot think of many benefits in doing so.
I have noticed that the true tick rate drops to about half when the game is not a focused window. I believe this to be the host computers operating system dealing with resource allocation. I am unsure why it would be appropriate to change that, or compensate by advancing more ticks while operating in the background. Depends on the goals and intentions, and a host of other issues.
The main reason for these changes is to make the timing system faster and more precise by dealing directly in high resolution timestamps and almost exclusively in in-the-moment math.

}
void Timing::setTickRate(int rate) {
tickRate = rate;
tickStep = ceil(1000000000.f / float(rate));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This runs into extreme floating-point inaccuracies. The exponent required to store anything close to this number is huge, the result is effectively an integer, which is only usable in this case by chance. This needs to be changed.

Copy link
Author

@PyroFire232 PyroFire232 May 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tickStep is the (integer) number of nanoseconds between frames, with 1,000,000,000 (1billion) being 1 second.
Fractions can be effectively thrown out.
I'll look up some integer size charts and do this properly, dependent upon intentions and goals for timing systems.
Based on your system, I think double is needed.

@@ -87,6 +114,9 @@ CBR::CBR(GraphicsResources* gr, const PGE::String& filename) {
reader.skip(2 * 3 * 4 + 5 * 16);
int vertexCount = reader.readInt();
int vertexOffset = vertices[textureID].size();

std::vector<int> indices;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this? Primitives are our indices.

Copy link
Author

@PyroFire232 PyroFire232 May 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a mash-up of both formats to get something functional.
There is a certain satisfaction to factually knowing it works in concept.
Further iteration required, as such is the case with the file format expecting further breaking changes, so I didn't want to do too much.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean? This variable is just declared and never used.

@juanjp600
Copy link
Collaborator

Precise tickrate and framerate handler (I fixed your timers, basically).

As far as I can tell you've done nothing of value to the Timing class, if anything you've made it more difficult to understand. Explain what your implementation solves.

<Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
<ImportGroup Label="ExtensionTargets">
</ImportGroup>
</Project>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we would be managing this and all other libraries as submodules.

Copy link
Author

@PyroFire232 PyroFire232 May 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was an issue with getting the targeting correct on those libraries.
There was also issues importing libogg.lib, libvorbis.lib and libvorbisfile.lib that should be looked at as well.
And also whether or not you want it a static lib, or a .dll. At the moment, they are static libs.

Font* World::getFontSubotypeLarge() const { return fontSubotypeLarge; }
Font* World::getFontSubotypeSmall() const { return fontSubotypeLarge; }
Font* World::getFontBoldLarge() const { return fontBoldLarge; }
Font* World::getFontBoldSmall() const { return fontBoldLarge; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget the trailing newline.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

Also noticed the small versions return the large versions.
Woops!
I have not ended up using these fonts for much because they're very unusual.
Not the best for GUI purposes.
Still good for show, however.
Perhaps i should put some instances in the test gui area of the pause menu?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Font loading should be generalized to allow for mods to have their own fonts.

materials[i] = new PGE::Material(shader, gr->getTexture(path + textureName));
}

meshCount = scene->mNumMeshes;
meshes = new PGE::Mesh*[meshCount];
originMaterials = new unsigned int[meshCount];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have a memory leak here. This array is not deleted at any point.

Copy link
Author

@PyroFire232 PyroFire232 May 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.
This array is used to store pointers to the original material for when a model is not given a custom material.

What concerns me is that I have not accounted for objects with multiple meshes that can be given distinct submaterials, for example, the clipboard and the paper on it are separate meshes, and the paper can be given the material of the document it contains.
This is a good example of where a generalized SetMaterial() and/or SetSubMaterial() may be superior to embedding it into the model.
I believe this is the multi-mesh and multi-material scenario that makes the "embed textures into a model mesh" idea unstable in certain potentially relevant circumstances.

@@ -15,7 +15,7 @@ class Camera {

float yawAngle;
float pitchAngle;
// The range of the the pitchAngle field before it is clamped.
// The range of the the pitchAngle field before it is clamped. - Gimbal lock workaround
Copy link
Collaborator

@juanjp600 juanjp600 May 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a gimbal lock workaround. This is simply because the camera in the game is deliberately clamped to a max pitch of 70 degrees, which we may or may not want to tweak. Without clamping, you'd be able to turn the camera well over 90 degrees and put it upside-down. Gimbal lock is not a concern with the player's camera.

Copy link
Author

@PyroFire232 PyroFire232 May 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think putting the camera upside-down happens when the player dies and looks straight up.
It may be appropriate to clamp this by other means.
Not sure. I've removed this comment.
The player camera in general needs more work.
I would be interested to know if you have any existing intentions or plans, or whether I can go ahead and do some experimentation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to do some experimentation, but be aware that much of camera and player movement should be dealt with in AngelScript, which probably requires exposing more functionality to it.

@@ -0,0 +1,23 @@
namespace Room { namespace rcbx { Template@ thisTemplate=Template();
Copy link
Collaborator

@juanjp600 juanjp600 May 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put namespace openers, any code following them, and closing braces on separate lines, like so:

namespace Room {
namespace RCBX {
Template@ thisTemplate=Template();
...
}
}

You can choose to not indent namespaces.

Copy link
Author

@PyroFire232 PyroFire232 May 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is done literally everywhere for a reason, but it can be removed later on.
I would like to open discussion about the coding style differences, and to describe for you where I am intending to follow style guidelines (but I may be short of the mark here and there), and where I am intentionally not following the guidelines.

I'm used to writing what I call quasi-encrypted compressed shorthand, as code is all just a's, b's and c's to me, but this is not at all conducive to a collaborative coding environment.
I am still intentionally writing compressed, however, for the same reason that I am using notepad and tabs for indentation in all .as files.

That reason is simple: To fit more code onto the screen at one time.
That is my biggest issue: When writing modules that are the size of these ones, it is really useful to be able to view and access all parts of the system in as small a screen space as possible. I don't want to have to scroll for 10 minutes, or click and drag the scrollbar and guess where something is, or ctrl+f through a bunch of generic names, because my program is filled with a bajillion practically empty lines that have a single statement on it, or worse, is simply a lone closing bracket. 3 lines are not needed for every foreach() { a(x); b(x); } This kind of thing is extremely wasteful on screen-space.
Abbreviating variables down to just "a" and "b" is done for the same reason, but I'm avoiding that because it does not work in collab, and equally for the same reason, too much compression would be non-conducive to collab as well.
I'm trying to find a good balance between compression for prototyping, and expanding for functions that do anything too varied or complicated.
At the end, I expect to completely decompress all lines and translate tabbing into the 4-space standard.

As for all other styling issues, I'm generally looking to follow them wherever I can.
Snake case was used because I was still learning the class system and how to lay it all out.
There's also one particular case where I am completely unsure: I think funcdefs should be defined as camelcase, as they are treated much like a class.

Lastly, I have questions about documentation, namely, how would you like that done?
Is there a particular format you'd like?
And what of code file comment headers, if any, i.e do you like having a description of what the script is and its purpose?

@@ -25,6 +25,7 @@ class PlayerControllerDefinitions : public RefCounter {

public:
PlayerControllerDefinitions(ScriptManager* mgr, RefCounterManager* rcMgr, Camera* cam);
float height;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be a public member.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is exposed to allow the game to control the camera height.
It should probably be done a better way.
Namely, i'm thinking of how the camera sinks into the ground in the pocket dimension room, and crouching.
No doubt there's a reason the player is a midget, so I didn't want to do too much, but being so tiny was really bugging me.
The desired camera height from the floor seems to be about 25 units.

delete perMenuFrameEventDefinition;
delete perLoadFrameEventDefinition;
delete perEveryFrameEventDefinition;
delete resolutionChangedEventDefinition;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A clearer explanation of these events is required.

Copy link
Author

@PyroFire232 PyroFire232 May 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Load Tick, and Load Frame, are a state that can be triggered by angelscript that causes it to only run those hooks.
It felt like a cleaner implementation for modding purposes, saves every mod doing if(Environment::Loading).
This should probably be further iterated into giving each mod its own loading states and cycles, particularly to account for dependencies, load order, and mods modifying the outputs of other mods.
Main reason to have this now is to build as though this state is separate from the primary tick and tickAlways functions, and particularly to reduce and/or manage initial load times in the immediate term, making it faster to test code iterations and changes.

For the other events;

Ticking;
updateTick happens immediately after tick++ happens. tick++ only happens if, and only if the game is not paused.
updateEveryTick happens immediately before the next tick++ happens.
It is set to trigger in this particular fashion (World.cpp:176) to allow advanceOneTick() to be created, such that the first EveryTick() unpauses the game, and marks the next EveryTick() to pause it again.
Some things also need to always tick, particularly the menus which are typically used while the game is paused, but also used during runtime like the HUD. This can be done in EveryTick().

Framing;
updateFrame is intended for frame advancement if, and only if the game is not paused, such as advancing animation sequences.
updateEveryFrame is intended for rendering things that should always be rendered, like the world, objects, and npcs in view.
I understand that framing is going to be done differently from what I have been thinking, and I have a suspicion that only updateFrame may be needed, where the engine may retain the world state and does not need additional render calls to continue rendering things in their current state.
updateMenuFrame is needed to render menus, as it also has a "RenderOnTop" context before it is executed. This is useful for menu animations too.

@juanjp600
Copy link
Collaborator

Added filepaths to top of root script files so assets can be moved out of the SCPCB folder later.

I do not understand the purpose of this. Why do you want to move assets out of the SCPCB folder?

@Saalvage
Copy link
Member

A few comments of my own:

  • The port of the rooms in ROOMS.rar to .cbr was by all accounts a total waste of time. As I said, not only will .cbr receive multiple breaking changes in the future, but we also intend to tweak all existing rooms, the progress of which can be seen in Resources/VMFs, that is as much of an asset overview as is needed.

  • The approach to software development you have demonstrated is not productive in a collaborative working environment. Completely rewriting parts of a system is absolutely valid practice, if, and only if the two following conditions are met:

    1. The existing code is flawed to a degree that it is unsalvagable, existing deficits are so severe that reworking the code to resolve them would require more time and effort than rewriting it.
    2. You are absolutely certain that the code you are going to write is vastly superior to the previously written code.

    I see neither point fulfilled with any of the rewrites you have undertaken.

    Especially if you are not certain about existing flaws and/or your ability to write superior code it is absolutely crucial to consult other developers to get a better understanding for the existing flaws in the code as well as to get first feedback on your planned approach, preferably before any code is written.

  • Your dismissive/uncooperative attitude is also not helpful.

    1. You were told that a RMesh importer for CBRE was not feasible.
    2. You were told that a pull request of this size is not workable.
    3. You were told that OpenAL would not be used.
  • I must also mention that your behavior in channels surrounding the game is worrisome. You are not yet part of the development team and only possess a very surface level understanding of the 1.4 update, yet still often behave as if you were. I completely understand you might be very excited about this project, but you should still reflect on what you are saying/doing. This includes commenting on a lot of GitHub issues of CBN with nothing meaningful to add.

I am aware that we also absolutely made the mistake of failing to properly communicate our goals, but ultimatively you should have inquired more about what we wanted and/or needed, moving forward.

@juanjp600
Copy link
Collaborator

The GUI changes seem to be at least conceptually okay. Some refactoring is certainly needed, primarily to address naming conventions and other code style issues.

@PyroFire232
Copy link
Author

PyroFire232 commented May 23, 2021

I do not understand the purpose of this. Why do you want to move assets out of the SCPCB folder?

This primarily stems from a desire to clean up the assets folders.
It feels strange having all of GFX, SFX etc immediately next to the script folder.
There's also some questions I have about how modding resource packs will be handled, and actually whether or not the base resources should go into said resource packs, effectively achieving what this change is meant to account for.
For now, it is more for debugging purposes to easily change how the filepaths and systems will work in final.

The GUI changes seem to be at least conceptually okay. Some refactoring is certainly needed, primarily to address naming conventions and other code style issues.

Fantastic!
I'll go ahead with some big cleanups on the GUI system and start on more completed and polished version, as well as clean up the code for the menus.
I'll likely add an Align::Corner*, as well as a GUI::Square object that is effectively a Rectangef but in the appropriate format, and move the GUI string functions into GUI::String of some sort.

@PyroFire232
Copy link
Author

PyroFire232 commented May 23, 2021

A few comments of my own:

* The port of the rooms in ROOMS.rar to .cbr was by all accounts a total waste of time. As I said, not only will .cbr receive multiple breaking changes in the future, but we also intend to tweak all existing rooms, the progress of which can be seen in Resources/VMFs, that is as much of an asset overview as is needed.

It was fun exploring the rooms from the rar in the gameworld.
Personally, that was worth the time compiling all the rooms into the new format.
Also found some missing textures, bonus!
914.room(). satisfying.

* The approach to software development you have demonstrated is not productive in a collaborative working environment. Completely rewriting parts of a system is absolutely valid practice, if, and only if the two following conditions are met:

I understand your perspective, however I disagree primarily where experimentation can lead to creative and effective solutions, as well as leading to a better understanding into how a system works and how it can potentially be made better.
I offered to help, you were open and seemed interested, but there's this sense that you have no idea what I am capable of, what I know, what my coding is like, and how effective I can be.
I'd say that the code here is a great demonstration of my abilities.
What scenario would we be in if this only contained some menus, or simply a few item definitions?

  1. The existing code is flawed to a degree that it is unsalvagable, existing deficits are so severe that reworking the code to resolve them would require more time and effort than rewriting it.

Any particular areas?
I offered to help, and it would have been an empty gesture without offering actual code.
A large part of what I'm doing involves learning and experimentation, and I have no doubt that some of what I am doing here will be beneficial or inspiring to the next iteration.
It is my opinion that it would be premature to dismiss this entirely in such a generalized way, but I want to assure that I understand where you're coming from.
This is actually why i've made a big description, because going further would require cementing changes that are not yet agreed. Proceeding further would be wasteful.
I've been doing stuff for about 2 weeks, so this is a good time to stop and take an account of what's been done and where to go next.
It may be appropriate to repackage some of the more desirable changes into something more polished and finalized, perhaps rebased on the root of the fork, so that it may be continued with.

  2. You are absolutely certain that the code you are going to write is vastly superior to the previously written code.

No human is capable of having such certainty.
There are pro's and con's to every approach, and a lot of it depends on things that have not been discussed.
If I stopped and waited for approval and confirmation for every detail, no code would be written.
When I first saw the discord, I felt as though this is the trap that the development cycle has been stuck in for some time; decisions not being made because there's a fear that the system will not be 'vastly superior' to the previous, while the only way to know that for sure is to prototype it.
Looking over the discord history, public interested seemed to be fading too, but it appears to be picking back up again as of late.
It would be quite an achievement if this game were to someday be featured at GDQ, and I think this goal is entirely achievable for the game, particularly with its roots in the SCP Foundation.

  I see neither point fulfilled with any of the rewrites you have undertaken.

The fact that the resource instancing system is entirely self contained, and is directly initialized and cast correctly on load, without searching for a particular class by string name, is vastly superior to your reflections system. This modular self-contained resource management system can be further adapted to deal with whatever modding interfaces you want to use.

  Especially if you are not certain about existing flaws and/or your ability to write superior code it is absolutely crucial to consult other developers to get a better understanding for the existing flaws in the code as well as to get first feedback on your planned approach, preferably before any code is written.

I have attempted to do this at length in the discord.
Without primary directions, I'm just playing, experimenting, and learning, building up from what I roughly anticipate is a new game driver for something that will roughly look and feel like the previous game, as well as looking for places where I can contribute genuine value.
In a sense, you don't know what to instruct me to do until you know what I can do, and I do not know what needs to be done, so I don't know what to ask about, at the same time, you've probably been busy with either parts of this project or other projects.

* Your dismissive/uncooperative attitude is also not helpful.

Hm.

  1. You were told that a RMesh importer for CBRE was not feasible.

Yet I managed to import a model into CBRE, and learn on a technical level what the problems were.
Additionally, I have also learned how model file formats work, importing/exporting and dealing with polygons, materials, file data etc. A lot of that is new to me, so feasible or not, I have learned something valuable.

  2. You were told that a pull request of this size is not workable.

And is precisely why I'm stopping now to take account of what's good in these changes, what is not, and to determine design directions from here.

  3. You were told that OpenAL would not be used.

Yet it appeared on the SCPCB git frontpage until I said something about it.
You were personally unsure when I first asked, too.
I'm glad we have settled that issue and are confident how the sound system will work now.
I also learned quite a lot about how each different sound system works in my experimentation with each system.

* I must also mention that your behavior in channels surrounding the game is worrisome. You are not yet part of the development team and only possess a very surface level understanding of the 1.4 update, yet still often behave as if you were. I completely understand you might be very excited about this project, but you should still reflect on what you are saying/doing. This includes commenting on a lot of GitHub issues of CBN with nothing meaningful to add.

I see your point about the comments in the github, I have removed them for you, apologies.

Regarding behavior in channels, two things;
First, I seem to be the only other person, aside from yourself and Juan, who are doing anything in terms of development on the next base game update. You both can't always be available, so it's nice there being someone to speak to who at least knows a something. I clearly have no developer tag, and I have referred to yourselves where necessary.

Second, I suffer from ADHD so challenging behavior is regularly an issue for me IRL, all I can do is humbly request your patience and understanding, and to assure you that I genuinely mean well.

I am aware that we also absolutely made the mistake of failing to properly communicate our goals, but ultimatively you should have inquired more about what we wanted and/or needed, moving forward.

With all due respect, it's been two years and you have little more than a working 3d engine, with no sound system, and a model editor. While these things are fantastic, there is little in the way of any actual game, so I have been focusing my efforts towards this end.
There is also a half-finished obsolete c++ source which appears to be a 1:1 refactor of the original game into a different language, appearing almost like it has been put directly through a translator.
My proposed changes have, without a doubt, clearly inspired yourselves regarding design decisions and directions which you appeared to be stuck on, and I appear to have motivated new ideas for design choices, and directions for further work, as seen by the recently opened issues on SCPCB/main following this review.
In this way, I am happy to have contributed something meaningful to development.
Even if you wish to see my code as useless or not worthwhile, it has certainly achieved something that I am proud of 👍

@juanjp600
Copy link
Collaborator

With all due respect, it's been two years and you have little more than a working 3d engine, with no sound system, and a model editor. While these things are fantastic, there is little in the way of any actual game, so I have been focusing my efforts towards this end.

I understand that we've been awfully slow here, and I appreciate your efforts. We're not asking you to stop trying to contribute, we just think that we need to communicate more so we can figure out what needs to be done, and how to go about it. As you've stated, some of your changes have inspired us to nail down some missing parts of the design, which is really useful.

There is also a half-finished obsolete c++ source which appears to be a 1:1 refactor of the original game into a different language, appearing almost like it has been put directly through a translator.

That's exactly what the OldSrc folder is. Originally the scope and goal of this project was very different, and the OldSrc folder was part of a plan that didn't pan out. We just keep it around in case there may be something genuinely useful in there, but we try not to copy its contents. Don't be discouraged by lost work, we have a lot of it already but we hope the end result will be much better than what we did originally.

@Saalvage
Copy link
Member

Saalvage commented May 23, 2021

Basically what juan said, I also greatly appreciate your contributions. The lack of activity can't be attributed to a lack of direction, but rather to an occupation with other stuff. Juan is busy with Barotrauma and I'm busy with my own projects as well, which is why not much has been happening concerning CBN.

It is my opinion that it would be premature to dismiss this entirely in such a generalized way, but I want to assure that I understand where you're coming from.

What I wrote was not directed at your code, but rather meant as one of the prerequisites necessary to justify a complete rewrite such as the one you did.

No human is capable of having such certainty.

If you're looking at your own code from 10 years ago I assume there's a 99% chance that you will have such certainty, at least such is probably the case for almost every developer.

Second, I suffer from ADHD so challenging behavior is regularly an issue for me IRL, all I can do is humbly request your patience and understanding, and to assure you that I genuinely mean well.

I completely understand, thanks again for your help, and let's work together to get this done!

So I think the most appropriate course of action would be to seperate the GUI changes into a seperate PR, do you think you could do that?

@PyroFire232
Copy link
Author

PyroFire232 commented May 23, 2021

Don't be discouraged by lost work, we have a lot of it already but we hope the end result will be much better than what we did originally.

Such is always the case.
Always throwing out old code.
Very much intended.

So I think the most appropriate course of action would be to seperate the GUI changes into a seperate PR, do you think you could do that?

In happening.
I'm removing a bunch of bloat and features I didn't end up using like the padding, margin is better, added GUI::Square math object, and added more documentation for what each variable does, and generally polishing it off to work as cleanly as possible.
Big differences between early prototypes and production quality versions.
Self contained this time, only replaces menus and hooks into the menu, and a tiny RootScript.as for testing purposes, saves external x1000 spam (I found that annoying too)

I completely understand, thanks again for your help, and let's work together to get this done!

Thank you. Let's do.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants