Refactor Unit

[nabaco]

As discussed in the last community meeting, we want to refactor the Unit classes, merging them together and simplifying the codebase. I'm still not very familiar with that codebase, so maybe @danielrh will be able to offer some more technical details on how this can be done.

[nabaco]

@BottledByte will you be willing to take this task upon yourself? Feel free to reject if you're dealing with other things right now.

[BottledByte]

I see that @danielrh is assigned to this task already, so I am not sure if that means that he is going to refactor it. If he is only gathering some info about it, I will gladly take this task myself, but some ideas on how to do this would be indeed useful. But I will try my best anyway :)

[Loki1950]

I do not think that danielrh can be very active ATM

[nabaco]

I have assigned this to Danielrh only for him to suggest some information on the implementation.

[Loki1950]

The Python scripts do a lot of accessing of the Unit class so close look at them may be productive

[danielrh]

I think there are 2 reasonable approaches a) make the types indistinguishable: this means pasting every member var thing inside GameUnit inside Unit then adding a typedef Unit GameUnit; so that the rest of vega strike can't tell the difference b) Modifying the game engine to take GameUnit everywhere. The only time "Unit" should appear is inside the unit_generic.hpp and game_unit.hpp headers. Once the game only talks about GameUnit and GamePlanet, etc.... at that point you can proceed to plan (a) by trying to merge the classes.

[BenjamenMeyer]

I like approach b personally.

[stephengtuggy]

I like approach b as well.

[royfalk]

How is this progressing?

[royfalk]

As I've not seen any response from @BottledByte , I've started working on a parallel course. I'm extracting code from Unit class to super classes. e.g. drawing code in Drawable. This has the advantage of making code easier to refactor and debug.

However, I'm at the point where I should also be making changes to other classes such as GameUnit. I'd appreciate either a status update from @BottledByte or a reassign from @BenjamenMeyer .

[BenjamenMeyer]

@royfalk agree; it's yours.

[royfalk]

A (not so) brief update of where I am right now. I finally figured how to change one thing in the unit area without breaking everything else. I switched from GameX : GameUnit<X> and X : Unit to GameX : GameUnit<DummyUnit> and deleted X, moving its code to GameX. This worked and was relatively painless to do. I will now do this for all components and submit a first PR.

In the second phase, I intend to refactor the unit area to the following:

• Drawing code in drawable class
• Normal movement code in movable class. I'm still thinking about how to implement immovable and orbiting classes.
• SPEC and jump will move to their own classes.
• create ship class and playership class from it.
• create station class.
• Refactor a whole bunch of utility classes into these objects. Right now, class methods are sometimes spread over multiple files.
• Rename GameX to X.

Third phase

• optimize code
• create cities, planets will own orbiting stations and cities.
• Assessor class to estimate damage repairs and unit cost.
• Components - shield generator, jump drive, etc.
• Implicit components - shield generator in capital ships.
• Shuttle for capital ships - this is just an image to paint in the docks and changing of the text to say something like 'transfer in shuttle to your ship' instead of 'launch ship'.

As you can see, it's a good chunk of the game and it will take me several months and multiple PR's. @BenjamenMeyer to decide if to spawn some of these to another issue. I am staying away from graphics and python.

I apologize if I've not been in touch. I've been working in my free time on this code and it was very difficult and time consuming. Any change to unit_generic now necessitates a recompilation of practically the whole game.

Any comments? Suggestions? Criticism?

[Loki1950]

Just a quick comment in the space context non-movable is nonsensical everything is in motion all the time the question really is is it perceivable to the so called observer always with in a defined frame of reference.

[BenjamenMeyer]

@royfalk that's fine; sounds like something that should be made into its own project and we can break down the various parts into separate issues tied together under the project. It can be a slow migration effort to it.

Note: As part of refactoring stuff, I'd like to break things down into smaller static libraries that we can tie together into the final executable. I don't expect to get there any time soon, and want to refactor the layout to reflect.

@royfalk we can minimize the need to recompile everything with how the headers are structured. Forward declaring can solve some of that so that the linker takes care of it in the end; we can then either have a precompiled header to do it globally or just do the relevant parts in the various implementation files.

[nabaco]

Wow @royfalk, that sounds like one hell of a project, good luck with it! Don't hesitate to get in touch in case you some help with this :) One thing I would to request: While you progress with this huge effort, please try to add as much documentation as possible in the code. By that I mean both Doxygen documentation, as well as general comments regarding different decisions you have made in the process.

[royfalk]

A few updates:

• Refactoring of planet is the hardest and moving forward.
• Two bugs I've encountered: the ship in doc is dark. I'm assuming it isn't lit by a source. The planet is reflecting too much light. I thought it was a lighting issue but if you fly around, it goes normal. I suspect it's some reflection value that's causing it.
• I'm also seeing some new errors during runtime. In general, we should clean the old ones so the new ones will pop.

Hopefully I'll sort it out this week.

@BenjamenMeyer - where do you want this PR? 8.0? 9.0? I'm fine with not going to master, but other commits will need to merge up to this to prevent an issue where in a few months we'll have a massive merge. Do we have a plan in place for "future" development?

[nabaco]

@BenjamenMeyer - where do you want this PR? 8.0? 9.0? I'm fine with not going to master, but other commits will need to merge up to this to prevent an issue where in a few months we'll have a massive merge. Do we have a plan in place for "future" development?

@BenjamenMeyer I think we need to make a series of small pull requests into master. IMO this change should go into 0.7.x branch.

[BenjamenMeyer]

In general we have a Backlog milestone a to track stuff that hasn't been assigned a milestone yet.

0.7.x is about refactoring IIRC; so I'm fine with it for there, otherwise 0.9.x as I'd like to keep 0.8.x to only Python 3 changes if possible ATM.

[royfalk]

1. Is anyone else seeing these error messages after planet creation: 0:75(14): error: syntax error, unexpected NEW_IDENTIFIER, expecting ')' or ','

Fragment Program Error: Failed to compile fireglass Compilation of technique fireglass failed... trying ../2_ps1.4/fireglass Cause: Error compiling program vp:"fireglass" fp:"fireglass" Compilation of technique ../2_ps1.4/fireglass `successful

2. Is the llama too dark in this shot. I think I disabled a light source or something. @danielrh can you comment on how/where in the code these are displayed?

3 Also, @danielrh do you know what can cause the atlantis effect. Based on these shots, I'd say something turned reflection from the sun to max.

Could either of 2/3 be related to point 1? Thanks.

[nabaco]

@royfalk where can we see your changes? If we'll have a look there maybe it'll be easier to give some insight

[royfalk]

@royfalk where can we see your changes? If we'll have a look there maybe it'll be easier to give some insight

I haven't pushed it yet since it's buggy. I can do it later today if you want.

[Loki1950]

The errors you first posted are the failure to compile of one particular shader not sure but if the shaders are borked do not expect any constancy in the the render pipeline

[royfalk]

@nabaco I've pushed to https://github.com/royfalk/Vega-Strike-Engine-Source/commit/d4a546d26feaa26f248821cd0bcd109dc29cd51f and some previous commits. As noted in the commit, I've identified three issues: 1. missile killing firing ship if it hits 2. Atlantis reflection 3. no light on docked ship

[royfalk]

I've partially diagnosed the missile issue. It's colliding with the firing ship as it is fired. Will investigate further tomorrow.

[Loki1950]

Have you looked at the interaction of the unit class with Python as most of the unit data is initialised though Python not C++

[royfalk]

So, this turned out to be just as awful as I had suspected. Barring a complete rewrite (yikes), it is almost impossible to rewrite in parts without breaking something. I'm now refactoring the interfaces such as system and universe. This serves two purposes: I understand the code better and it is easier to modify. Hopefully, I'll get back to this in about a month.

[BenjamenMeyer]

@royfalk sounds good; the refactoring of the other parts looks to be good and is stuff we need to do any way. Thanks!