Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge DIS7 (CSharpDis7) in the same assembly #13

Open
mastertnt opened this issue Oct 11, 2021 · 12 comments
Open

Merge DIS7 (CSharpDis7) in the same assembly #13

mastertnt opened this issue Oct 11, 2021 · 12 comments

Comments

@mastertnt
Copy link

In order to be able to manage multiple versions of DIS in a 3rd party application, the source "CSharpDis7" must be merged with the common library OpenDIS.

In order to achieve that, the CSharpDis7 must be changed/rewritten to conform with the common methods used in OpenDis.

@leif81
Copy link
Member

leif81 commented Nov 19, 2021

I've wondered many times now how best to maintain the two parallel Dis6 and Dis7 versions. It seems fairly unsustainable to keep and be patching both. This problem extends to the other language implementations like Cpp and Java. I'm open to ideas.

My understanding is the Dis7 implementation should be backwards compatible with the Dis6 spec, so maybe we work towards dropping the Dis6 implementation. One concern I have with that is more testing and fixes have gone into the Dis6 implementation so those fixes we'd want to bring forward into the Dis7 implementation before dropping it.

@Schreck425
Copy link

Any chance someone can explain in a couple sentences how to use the CSharpDis7 folder? I don't know how to compile the DIS version 7 and don't see any instructions on how to.

@leif81
Copy link
Member

leif81 commented Jan 10, 2023

@Schreck425 To my knowledge no one has documented steps to compile the DIS v7 code yet. The CSharpDis7 folder of code was autogenerated (and so was the v6 code). Most effort to date has been focussed on compiling and stabilizing the v6 code. To get the v7 code compiling and steps documented it will require a volunteer. Feel free if this interests you. A pull request for this would be welcomed.

@Schreck425
Copy link

@Schreck425 To my knowledge no one has documented steps to compile the DIS v7 code yet. The CSharpDis7 folder of code was autogenerated (and so was the v6 code). Most effort to date has been focussed on compiling and stabilizing the v6 code. To get the v7 code compiling and steps documented it will require a volunteer. Feel free if this interests you. A pull request for this would be welcomed.

I would if I was qualified, but I'm struggling to figure it out unfortunately. I don't understand where those files were autogenerated from. I don't get too far. I see the solution has a 1995 and 1998 folder with autogenerated code that compiles, so for grins I created a 2012 folder and copied in those Version7 autogenerate files to see if I could use the DISnet namespace and come even close to compiling but confirmed quickly that I really don't know what I'm doing. Kind of hard for me to make documentation for the documentation I need in the first place, lol.

@agerrius
Copy link

agerrius commented Sep 5, 2023

I have started looking at this issue and made some tests moving the DIS7 code into the DIS5/6 assembly.

But I also notice that the DIS7 code is missing some common PDUs, like Transmitter and Signal PDUs. Does anybody know how this code was generated and why some PDUs are not there?

@agerrius
Copy link

agerrius commented Sep 7, 2023

In issue #14 a solution to put everything in one assembly is basically already provided:

#14 (comment)

Would it not make sense to merge that change into the master? It is more useable than having DIS7 separate as it is now?
What I did as a test is the same approach that has been done there already.

What remains is the issue that the DIS7 implementation is not complete, but that is the topic of #14.

In #14 there is also some discussion by @Schreck425 if it would make sense to have a common base class for each PDU type over the different versions (#14 (comment)). I guess that issue could been seen as part of moving it all in one assembly. On one hand that would make supporting multiple DIS versions in an application easier, on the other hand it could also make it confusing for users if the changes between versions are bigger.

I had a look at the OpenDIS implementations for the other languages and it seems all of them threat each DIS version as something separate. None seems to have a generic PDU object that can support multiple versions. So I guess a question is then also if the C# implementation wants to use a very different strategy. Does anybody have thoughts on that?

@agerrius
Copy link

I would like to bump this discussion again. With the code that @Schreck425 provided in #14 the DIS7 is in the same assembly now. But trying to use different DIS versions in one application is not very convenient with it. This is because all data type are defined per DIS version. So for example there is a Vector3Float in the Dis1998 namespace and another one in the Dis2012 namespace. This means there is quite some duplication in an application try to support both these versions, as the similar (but different) types needs to be supported.

Would it not make sense to move the more generic datatypes that are shared over DIS versions out of the namespace of the specific DIS version and somewhere in the core of the library?

Like I mentioned above the OpenDIS implementations for other languages also seem to threat each version as completely different, so I am not sure if it would be a good idea to have the C# version work differently. Would be welcome to hear thoughts on that.

@Schreck425
Copy link

Yes unfortunately there's a lot of repeated code which is not good. It seems that some of Dis7 was auto generated and maybe some was not? Some of it was at least not correct at all and I started manually fixing things as I went.

It seems that a lot of Dis 6 and 7 values are the exact same thing but with different names. That's what it looked like according to my documents at least. I agree there should be a common library between the versions where they are identical.

I also have a conversion program and when I made that it showed how similar it all is. If we made a common library, do you name the values what they're called in Dis6 or Dis7?

It's a mess unfortunately but it got done what I needed.

@agerrius
Copy link

Yes, some names have changed indeed. I just compared the EntityState for the DIS 6 and 7 in the standard and the articulated parts are indeed renamed to variable parameters. So in that sense the changes in the generated code are correct. And also the fact that the Site and Application ID are not combined in a Simulation address is indeed a change in DIS 7. But the underlaying data is the same (for the variable parameters there has been a change in the structure of the underlaying data as well).

I guess it is good to use the names from the standard exactly as they are. But if the underlaying data is the same, it would also just be possible to have a different Field that access the same data of the object via a different name. That does not have to require a new class. But not every change is that simple of course.

I'll do some more analysis of the changes and see if I can come up with some proposals to simplify things when using multiple versions.

For our application I decided to leave most code in DIS 6 as it was. And I have added a conversation utility class that can translate incomming DIS 7 PDUs (for the types we need) to their DIS 6 equivalent. That way I had to change less in our application (or had to create less duplications).

@Schreck425
Copy link

We basically need to do something like this right? We need to pull out all of the common stuff and have a common class.

public class Dis6 {
    public SomeEqualParamater myDis6Name;
}

public class Dis7
{
    public SomeEqualParamater myDis7Name;
}

// This is the same data on Dis6 and Dis7
public class SomeEqualParamater { 
}

public class Tester {

    private Dis6 myDis6 = new Dis6();
    private Dis7 myDis7 = new Dis7();

    private void test() {

        // They still are the same value from a shared file
        myDis6.myDis6Name = myDis7.myDis7Name;
    }
}

@agerrius
Copy link

agerrius commented Mar 8, 2024

Hi,

I have made one branch with the improvements I made to the PduFactory and PduProcessor. This branch can be found here:

https://github.com/agerrius/open-dis-csharp/tree/Dis7_improvements

I have started in another branch to try to use common data type. This is still very much work in progress, but I tried to derive all Pdus (from all versions) from the same base class. Also for each Pdu type (e.g. Fire PDU) I want to have a common base class over all DIS versions. That way I think processing will be easier in applications. I have now implemented it for the warfare PDUs and plan to try the entity information family next.

You can find the current status here, but I will post a message when I am more done with it:

https://github.com/agerrius/open-dis-csharp/tree/dis7-common-datatypes

But before I continue I plan to first make some unit tests to make sure that my changes don't break existing functionality or loose attributes by accident.

@leif81
Copy link
Member

leif81 commented Mar 9, 2024

Yes unfortunately there's a lot of repeated code which is not good. It seems that some of Dis7 was auto generated and maybe some was not? Some of it was at least not correct at all and I started manually fixing things as I went.

The DIS 6 and DIS 7 code bases here were originally auto-generated with xmlpg. This is the same tool that was used to generate Java, C++, Python, JavaScript implementations of Open Dis.

Given the backwards compat between DIS 7 and 6 specs, I'm becoming more and more convinced in my own head to that merging the DIS 6 and 7 code bases into one DIS 7 code base is the best way out of this mess. And my suggestion would be to apply the same approach for all the Open Dis projects (Java, C++, etc). If anyone sees any major issues with this approach I'm very interested in hearing.

Merging the 6 and 7 code bases will be non trivial because a lot of by-hand fixes were done to DIS 6 (especially in the Java and C++ libraries) that will need to be brought over to DIS 7 code base. However that would still need to be done at some point anyways to reach a stable DIS 7 implementation. To mitigate this risk to DIS 6 users we could leverage the existing unit tests to ensure DIS 6 PDUs are still being processed with DIS 7.

The upside of taking on this large effort is once done we could mark the DIS 6 code base deprecated and later remove it from the repository so that we can focus all efforts in single DIS 7 implementation for each language.

DIS 8 is a whole other question mark. Given that it's planned to NOT be backwards compat. I have many questions about the value of doing so and why SISO is entertaining such an approach. I have heard the answers, but still have many concerns. Breaking multi decade compatibility on a stable spec used by many nations and many applications seems to me like a really really unwise thing. I digress.

My crystal ball is DIS 7 will be what the community will use for the next couple decades, and adoption of DIS 8 will be slow and take decades. Given that assumption, anyone building DIS simulation in the next couple decades will benefit from a good implementation of DIS 7, so this large work proposed above would be of significant value.

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

No branches or pull requests

4 participants