0003511: Turok's CRC code: Missing BSD license

Notes
(0019757) Pol M (developer) 2018-09-25 16:05	Should it be appended to the current licence?

(0019760) WubTheCaptain (reporter) 2018-09-25 18:17	Quote from Pol M Should it be appended to the current licence? I believe so, in another comment.

(0019761) WubTheCaptain (reporter) 2018-09-25 18:23	I believe one also needs to reproduce the copyright notice in the binary distribution somehow. Probably at Help → About → Plugins → Turok 2 Remaster.

(0019762) Pol M (developer) 2018-09-25 18:33 edited on: 2018-09-25 18:34	Ok, I'll look how to make it in. I'd take the style from Help -> About (the main textbox) with a link to to the page attached to this ticket. Or maybe something fancier like the JSON licence if you want (If you think that this second option is the correct one, please tell me so).

(0019763) Zalewa (developer) 2018-09-25 20:47 edited on: 2018-09-25 20:48	Pol, I agree with you that going with the textbox is the correct approach. Add it to the plugins tab in the About dialog and you should be able to populate it with any text the plugin provides. The text itself, however, is another thing altogether. There's currently no API to provide such text. You could see how the name and version are inserted. The text that appears there is set in the init() call (example: chocolatedoomengineplugin.cpp, look for init() call in the constructor). However, any data that is generated there in this init() is generated only once and also before the translations are loaded and then it is never generated again. This means that if you add an extra "QString aboutText" field to EnginePlugin::Data, it will never get translated. Solutions to this can be: 1) Around-the-bush and boiler-platey, but doesn't break ABI: 1.1) Create a "class MAIN_EXPORT TextProvider : public QObject { Q_OBJECT; public: virtual QString provide() = 0; }". Place this somewhere in the serverapi in a textprovider.h file. 1.2) In plugins, create [PluginName]AboutProvider : TextProvider classes, override the 'provide()' method and return the tr()'d about text from there. 1.3) In EnginePlugin::Data create a "QSharedPointer<TextProvider> aboutProvider" field. 1.4) In init() in each plugin, assign "new [PluginName]AboutProvider" to the "Data::aboutProvider" field. 2) More straight-to-the-point, but will break ABI. Note that while this is unprofessional, it has also been very common through the course of the development of this program and also all plugins will require update for the next release anyway. 2.1) Add a "virtual QString aboutText();" method to EnginePlugin with default implementation returning an empty "QString()" object. 2.2) Somewhere in Turok 2 Ex plugin create a localized class that inherits from QObject and utilizes Q_OBJECT macro so that tr() can work. You can call this macro "Turok2ExText". This class should work just fine even when declared in the turok2exengineplugin.cpp file. 2.3) In this class add "public static QString about()" method from which you will return the plugin's about text. 2.4) In Turok2ExEnginePlugin override the method "QString aboutText()" method and make it return the value produced by "Turok2ExText::about()". I'll leave the final choice to you, or maybe you could come up with something better.

(0019766) Pol M (developer) 2018-09-25 21:09	Okay,this will require a little bit of time. 😂 While both sound correct to me, the first route sounds more like stuff I've barely touched. That's the one I'll probably gonna try. Tomorrow/in two days I'll look more closely into it. Quote from Zalewa or maybe you could come up with something better. While not impossible, I trust your knowledge over the project. Thanks for the guidance, my Lord!😉

(0019767) Zalewa (developer) 2018-09-25 22:03 edited on: 2018-09-25 22:07	Usually the first solutions that one comes up with aren't the most optimal ones and through subsequent iterations you can find something better. With regards to ABI and why changes to API cannot be simple: the unfortunate problem of C++ is that you not only need to remember that breaking API (the code interface shared between main program and libraries) will result in a "segmentation fault crash" break of your code. The same crashy-crash problem, although much more insidious one, is caused by breaking ABI. To break the ABI one only needs to alter the size of the structure. How can one do that "unknowingly"? For example by adding a virtual method to the base class, which will alter the hidden vtable and change the size of the parent class, breaking all classes that inherit from it. See here:'https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B [^]' If you inspect the classes that have the MAIN_EXPORT tag you will see that there are some strange POLYMORPHIC macros present there and that the methods are documented as "virtual" or "pure virtual" despite missing the "virtual" keyword. This is polymorphism implemented through method pointers hidden away in Pimpl (Private Implementation, DPtr, DClass) which allows us to add new virtual methods to the MAIN_EXPORT classes without altering the vtable and thus without breaking the ABI. Thanks to this, adding a new method is quite a nuisance, but at least the program doesn't crash if you deploy new version of doomseeker.exe with old plugins.

(0019771) WubTheCaptain (reporter) 2018-09-26 00:27	Quote from Zalewa I agree with you that going with the textbox is the correct approach. Add it to the plugins tab in the About dialog and you should be able to populate it with any text the plugin provides. Created ticket 0003523 for supporting this. Please discuss there.

(0019797) Pol M (developer) 2018-09-28 20:06 edited on: 2018-09-28 20:07	Since the code to add the "description" of plugins is being reviewed (at the time of writing. please note that for the moment I have not made a pull request) I think that it's about time to ask what should be put in the About menu. I know near to nothing about licences (after seeing you discuss ticket 0003512), I only know the bare minimum. It would be nice if either someone else with more knowledge wrote down the text that should go into the menu(s), or that someone gave me a little bit (a lot) of guidance of where to start. (I don't even know what licences the plugins use)

(0019799) WubTheCaptain (reporter) 2018-09-29 00:09	Quote from Pol M I think that it's about time to ask what should be put in the About menu. See "Additional Information" for the text of the license that should go in (minus the introduction, only the part after "Text of BSD license is provided below:").

(0019809) Zalewa (developer) 2018-09-29 09:19 edited on: 2018-09-29 09:48	It's also necessary to take care into not misleading the users that the entirety of the plugin is on BSD, as the plugin is actually on LGPLv2.1+. If we just present the BSD license text into the about box, user may be mislead into thinking that the whole plugin is on BSD. I propose contents like this: Turok 2 EX - Doomseeker plugin This program is distributed under the terms of the LGPL v2.1 or later. CRC code on BSD License: <license text goes here>

(0019810) Pol M (developer) 2018-09-29 09:32 edited on: 2018-09-29 09:33	Quote from Wub See "Additional Information" for the text of the license that should go in (minus the introduction, only the part after "Text of BSD license is provided below:"). Yeah, I mean that what should be set for the rest of the plugins. I can leave it blank for the other plugins until for the moment if needed.

(0019811) Pol M (developer) 2018-09-29 09:34	Quote from Zalewa Turok 2 EX This program is distributed under the terms of the LGPL v2.1 or later. CRC code on BSD License: <license text goes here> I'll take this approach.

(0019813) Zalewa (developer) 2018-09-29 09:50	Err, make sure to state "Turok 2 EX - Doomseeker plugin" in the first line as per the edited comment 0003511:0019809. We don't want to ambiguously state that the game is on LGPL.

(0019816) Pol M (developer) 2018-09-29 12:46 edited on: 2018-09-29 13:01	Ok! You also suggested the addition to links. In the case of Turok, should I put the steam page? I could not find a webpage for the game. (or not put a link at all)

(0019833) Zalewa (developer) 2018-09-29 21:16	Why not GOG page instead? :) But no, I couldn't find any reasonable homepage for Turok neither, so sadly there should be none for this game. We shouldn't endorse shops.

(0019841) Pol M (developer) 2018-09-30 11:29	pull request

(0019842) Zalewa (developer) 2018-09-30 11:38	If Linda agrees with your changes, we can merge them and close this ticket as resolved.

(0019845) Zalewa (developer) 2018-09-30 19:05	Quote from Zalewa If Linda agrees with your changes, we can merge them and close this ticket as resolved. Dammit, I've been hasty and merged this already. Well, if something is wrong, there's always another commit...

(0019847) Pol M (developer) 2018-09-30 19:38	Quote from Zalewa Dammit, I've been hasty and merged this already. 😂Well, then I should mark this as "Needs testing?"😂

(0019848) WubTheCaptain (reporter) 2018-10-01 02:51 edited on: 2018-10-01 02:55	Trivial changes desired in Turok2AboutProvider::provide(). Quote "All rights reserved.\n" "All rights reserved.\n\n" Quote - Redistributions of source code ... - Redistributions in binary form ... 1. Redistributions of source code ... 2. Redistributions in binary form ... Yes, the URL in OP uses an unordered list. The source code uses unordered list, except asterisk characters () instead of ordered list. Not copyright-critical, but a trivial pedantic thing for consistency. 'https://directory.fsf.org/wiki/License:BSD-2-Clause [^]' 'https://spdx.org/licenses/BSD-2-Clause.html [^]' Quote* "This plugin is distributed under the terms of the LGPL v2.1 or later.\n\n" "CRC code on BSD License:\n"); These two lines should be two different translations, not one. (Don't merge it into the BSD-2 clause tr(), please. There should be four tr()s in total.) Quote from src/plugins/turok2ex/crc.cpp //------------------------------------------------------------------------------ // Copyright (C) 2015 "Samuel Villarreal" //------------------------------------------------------------------------------ Remove //-- on both top and bottom Move the remaining "Copyright (C) 2015 "Samuel Villarreal" text above "This library is free software;" Insert an empty newline between "Copyright (C)" and "This library is free software;" Have an empty newline between the two copyright notices and licenses. You should end up with something like this (shorter in this example for brewity): //------------------------------------------------------------------------------ // crc.cpp //------------------------------------------------------------------------------ // // Copyright (C) 2015 "Samuel Villarreal" // // This library is free software; ... // // This library is distributed in the hope that it will be useful, // ... // // You should have received a copy ... // Copyright (c) 2004-2006 Intel Corporation. // All rights reserved. // // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions are met: // // ... You may add //-- between the two licenses if you think it improves readability. (I think it's excess and worse for accessibility, especially for screen reader software.) Quote Copyright (C) 2018 "Pol Marcet Sardà" <polmarcetsarda@gmail.com> For future reference, quotes around your name are unnecessary unless it's a pseudonym. I'm aware the Turok 2 plugin uses them for Samuel's name, however rest of the code and common practice in software development does not.

(0019849) WubTheCaptain (reporter) 2018-10-01 03:04 edited on: 2018-10-01 03:08	Quote Copyright (C) 2018 "Pol Marcet Sardà" <polmarcetsarda@gmail.com> Also since we're dealing with a big new contribution here, it'd be best for you to disclaim any work-for-hire your potential employer may do on your code. In other words, if you work in software development and have a contract with your employer saying the employer gets assigned copyright on all your software development, ask them to sign off your code. See appendix of the LGPLv2.1 license, section "How to Apply These Terms to Your New Libraries".

(0019861) Pol M (developer) 2018-10-01 15:40 edited on: 2018-10-01 16:08	Quote from Linda In other words, if you work in software development and have a contract with your employer saying the employer gets assigned copyright on all your software development, ask them to sign off your code. Okay, if I'm understanding this correctly, in case that I work for a company I should add something like this: Quote from LGPLv2.1 You should also get your employer (if you work as a programmer) or your school, if any, to sign a "copyright disclaimer" for the library, if necessary. Here is a sample; alter the names: Yoyodyne, Inc., hereby disclaims all copyright interest in the library `Frob' (a library for tweaking knobs) written by James Random Hacker. signature of Ty Coon, 1 April 1990 Ty Coon, President of Vice The problem is that I do not work for anybody. I'm studying at an institute (not university/college) (they literally have no relation with any of this since it has been written in my free time and they have no classes in computer engineering.) If you insisted that this is necessary, I can ask, but I can guarantee you that they are not interested. (I haven't failed an exam in my life, I'm just young)

(0019863) Zalewa (developer) 2018-10-01 17:06 edited on: 2018-10-01 17:08	I risk derailing this ticket a bit, but I'd like to mention that you should be wary of such contracts (SIGN THIS CIROGRAPH AND ALL YOU DO BECOMES OURS) when you seek any job in the future. Your employer should not be interested with software you develop in your own free time, as long as you don't infringe on their rights (like using company's code) or replicate the software, though with that last one it can be rather vague what exactly constitutes "a replica" or "a software in the same realm". I don't know how the matter of such contract would hold up in court, but there are enough jobs in IT that you can safely stay away from contracts like this. If you get a chance to get a job as a student when nearing the end of your course, then take it - there's a high chance that they'll pay you, they'll allow you to prioritize finishing the studies over the job, they'll keep you after you get your degree and you'll skip job interview (I was that lucky 8 years ago).

(0019864) WubTheCaptain (reporter) 2018-10-01 17:15	Pol M: Just a note to us would've been fine. Since you seem not have an external relationship, I have no more concern with the subject. I'll await changes proposed.

(0019865) Pol M (developer) 2018-10-01 17:18 edited on: 2018-10-01 17:32	(Thank you for all your wise and kind words) Quote from Zalewa you should be wary of such contracts (SIGN THIS CIROGRAPH AND ALL YOU DO BECOMES OURS) when you seek any job in the future. Do not worry, I will. That clarified, I'll make a pull request with the suggestions, except the last one. Wich I do not think applies to my case. EDIT:'https://bitbucket.org/Doomseeker/doomseeker/pull-requests/31/adress-suggestions-related-to-ticket-3511/diff [^]'

(0019870) WubTheCaptain (reporter) 2018-10-01 17:43	Quote from Pol M https://bitbucket.org/Doomseeker/doomseeker/pull-requests/31/adress-suggestions-related-to-ticket-3511/diff I don't care to fix it, but lines 54–55 would've fit on one line now. Looks good to me, thanks! Zalewa, please merge.

(0019872) WubTheCaptain (reporter) 2018-10-01 17:53	Quote from Pol M It was done intentionally to be easier to read. Yeah, I can agree on that. Hence I did not bother requesting changes. 😄

(0019875) Pol M (developer) 2018-10-01 18:36	Commited:https://bitbucket.org/Doomseeker/doomseeker/commits/3ca4ea5f9b45

This issue is already marked as resolved. If you feel that is not the case, please reopen it and explain why.
Supporters:	No one explicitly supports this issue yet.
Opponents:	No one explicitly opposes this issue yet.

Issue History
Date Modified	Username	Field	Change
2018-09-22 23:12	WubTheCaptain	New Issue
2018-09-22 23:12	WubTheCaptain	Relationship added	child of 0003237
2018-09-24 01:18	Blzut3	Target Version	=> 1.2
2018-09-25 16:05	Pol M	Note Added: 0019757
2018-09-25 16:06	Pol M	Assigned To	=> Pol M
2018-09-25 16:06	Pol M	Status	new => assigned
2018-09-25 18:17	WubTheCaptain	Note Added: 0019760
2018-09-25 18:23	WubTheCaptain	Note Added: 0019761
2018-09-25 18:33	Pol M	Note Added: 0019762
2018-09-25 18:34	Pol M	Note Edited: 0019762	View Revisions
2018-09-25 20:47	Zalewa	Note Added: 0019763
2018-09-25 20:48	Zalewa	Note Edited: 0019763	View Revisions
2018-09-25 21:09	Pol M	Note Added: 0019766
2018-09-25 22:03	Zalewa	Note Added: 0019767
2018-09-25 22:07	Zalewa	Note Edited: 0019767	View Revisions
2018-09-26 00:24	WubTheCaptain	Relationship added	parent of 0003523
2018-09-26 00:27	WubTheCaptain	Note Added: 0019771
2018-09-28 20:06	Pol M	Note Added: 0019797
2018-09-28 20:07	Pol M	Note Edited: 0019797	View Revisions
2018-09-29 00:09	WubTheCaptain	Note Added: 0019799
2018-09-29 09:19	Zalewa	Note Added: 0019809
2018-09-29 09:32	Pol M	Note Added: 0019810
2018-09-29 09:33	Pol M	Note Edited: 0019810	View Revisions
2018-09-29 09:34	Pol M	Note Added: 0019811
2018-09-29 09:48	Zalewa	Note Edited: 0019809	View Revisions
2018-09-29 09:50	Zalewa	Note Added: 0019813
2018-09-29 12:46	Pol M	Note Added: 0019816
2018-09-29 13:01	Pol M	Note Edited: 0019816	View Revisions
2018-09-29 21:16	Zalewa	Note Added: 0019833
2018-09-30 11:29	Pol M	Note Added: 0019841
2018-09-30 11:29	Pol M	Status	assigned => needs review
2018-09-30 11:38	Zalewa	Note Added: 0019842
2018-09-30 19:05	Zalewa	Note Added: 0019845
2018-09-30 19:38	Pol M	Note Added: 0019847
2018-10-01 02:51	WubTheCaptain	Note Added: 0019848
2018-10-01 02:51	WubTheCaptain	Status	needs review => assigned
2018-10-01 02:55	WubTheCaptain	Note Edited: 0019848	View Revisions
2018-10-01 02:55	WubTheCaptain	Note Edited: 0019848	View Revisions
2018-10-01 03:00	WubTheCaptain	Status	assigned => feedback
2018-10-01 03:04	WubTheCaptain	Note Added: 0019849
2018-10-01 03:04	WubTheCaptain	Status	feedback => assigned
2018-10-01 03:07	WubTheCaptain	Note Edited: 0019849	View Revisions
2018-10-01 03:08	WubTheCaptain	Note Edited: 0019849	View Revisions
2018-10-01 03:08	WubTheCaptain	Note Edited: 0019849	View Revisions
2018-10-01 15:40	Pol M	Note Added: 0019861
2018-10-01 16:08	Pol M	Note Edited: 0019861	View Revisions
2018-10-01 17:06	Zalewa	Note Added: 0019863
2018-10-01 17:08	Zalewa	Note Edited: 0019863	View Revisions
2018-10-01 17:15	WubTheCaptain	Note Added: 0019864
2018-10-01 17:18	Pol M	Note Added: 0019865
2018-10-01 17:32	Pol M	Note Edited: 0019865	View Revisions
2018-10-01 17:43	WubTheCaptain	Note Added: 0019870
2018-10-01 17:43	WubTheCaptain	Status	assigned => needs review
2018-10-01 17:53	WubTheCaptain	Note Added: 0019872
2018-10-01 17:55	WubTheCaptain	Note View State: 0019872: private
2018-10-01 18:36	Pol M	Note Added: 0019875
2018-10-01 18:40	Pol M	Status	needs review => needs testing
2018-10-02 05:28	WubTheCaptain	Status	needs testing => resolved
2018-10-02 05:28	WubTheCaptain	Fixed in Version	=> 1.2
2018-10-02 05:28	WubTheCaptain	Resolution	open => fixed
2018-10-02 05:34	WubTheCaptain	Note View State: 0019872: public
2018-10-27 22:54	WubTheCaptain	Status	resolved => closed