SonyWWS / ATF

Authoring Tools Framework (ATF) is a set of C#/.NET components for making tools on Windows. ATF has been in continuous development in Sony Computer Entertainment's (SCE) Worldwide Studios central tools group since early 2005. ATF has been used by most SCE first party studios to make many custom tools such as Naughty Dog’s level editor and shader editor for The Last of Us, Guerrilla Games’ sequence editor for Killzone games (including the Killzone: Shadow Fall PS4 launch title), an animation blending tool at Santa Monica Studio, a level editor at Bend Studio, a visual state machine editor for Quantic Dream, sound editing tools, and many others.
Apache License 2.0
1.89k stars 262 forks source link

Remove unused variables and declarations #30

Closed hach-que closed 9 years ago

hach-que commented 9 years ago

Mono detects unused variables and private field declarations as warnings. Because ATF has "warnings as errors" enabled, these issues appear as errors and prevent compilation.

This PR removes unused / dead code, unused variables and unused private declarations. In scenarios where the declaration is required (e.g. in one particular Mutex case), I've added a #pragma to disable the warning instead.

Ron2 commented 9 years ago

Thank you very much for helping us clean up our code. I'm embarrassed to say that it's been on my task list for a long time now, to use ReSharper to go through all the code systematically and do clean-ups that Visual Studio doesn't help us do by itself.

I've reviewed most of the changes and they all look good so far. I'll get this pull request locally integrated and tested and reviewed. Sometime next week, I should have the changes integrated into master.

Going forward, I wish I had a way to automatically catch these warnings so that we don't accidentally break your project. We could adopt a whole Mono build process, but that might be a lot of maintenance. At the very least we could add a manual step to our major release process (every six months) where we use ReSharper to catch these warnings. If you have any tips, please let me know!

Also, it's exciting that you're using ATF with Mono! Can you tell me anything about your project? Is it on OS X? Using WinForms? Did you have to work around our Win32 dependencies? What about Direct2D?

Thanks again for your improvements.

hach-que commented 9 years ago

My plan is to get ATF working on Linux so that the Level Editor etc. can work cross platform. This is so that I can provide a better asset management tool and level editor for http://protogame.org/.

These changes don't actually get it fully building yet as there's some Windows-specific dependencies which don't resolve under Linux.

The next step for me is to clean up and re-organise the projects to support multiple platforms, which I'll file an issue later for so we can discuss the best way of doing so.

Ron2 commented 9 years ago

That sounds like a very cool project.

I have a question about this one change to SourceControlRevision.cs:

@@ -77,7 +77,7 @@ public DateTime Date
        {
            get
            {
                if (m_kind != SourceControlRevisionKind.Date || m_dateTime == null) //original line
                if (m_kind != SourceControlRevisionKind.Date) //new line
                    throw new InvalidOperationException("This revision is not a Date");
                return m_dateTime;
            }

The change removes the check for m_dateTime being null. I can see how we should probably be checking this in the setter, but I don't understand how this is a compiler warning. Did you have to remove this check for some other reason?

--Ron

hach-que commented 9 years ago

m_DateTime is of type DateTime, which is a value type and hence can never be null. The compiler was warning about an expression which is always false.

Ron2 commented 9 years ago

Ahh, thanks. I'm surprised that ReSharper didn't catch this.