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

feat: Few classes, member functions, many event sources, some static utility functions, Papyrus None as array fix #264

Closed
wants to merge 19 commits into from

Conversation

LarannKiar
Copy link

BSScriptUtil.h and Variable was edited to allow passing None to a Papyrus array.

Declared many event sources in BSTEvent.h.

Added classes ProcessLists, BGSStoryTeller.

Edited INISettingCollection to allow case insensitive searching for Settings. (Had some issues with finding settings like "SAddItemtoInventory").

Added Utility.h. It contains several native functions.

@ThirdEyeSqueegee ThirdEyeSqueegee changed the title Few classes, member functions, many event sources, some static utility functions, Papyrus None as array fix feat: Few classes, member functions, many event sources, some static utility functions, Papyrus None as array fix Aug 14, 2024
Comment on lines +20 to +25
RE::BSTArray<RE::TESQuest*> queuedToStart;
RE::BSTArray<RE::TESQuest*> runningQuests;
RE::BSTArray<RE::TESQuest*> queuedToStop;
std::uint8_t unk70[0x10];
RE::BSTArray<RE::TESQuest*> unk78;
RE::BSTArray<RE::TESQuest*> unk88;
Copy link
Contributor

Choose a reason for hiding this comment

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

remove RE:: they're already in the RE namespace

@@ -19,14 +20,28 @@ namespace RE
return *singleton;
}

[[nodiscard]] Setting* GetSetting(const std::string_view a_name)
std::string ToLowerStrV(std::string_view _sourceStringView)
Copy link
Contributor

Choose a reason for hiding this comment

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

ideally this should be moved somewhere else, but for now it can just be made private as it's an implementation detail.

Comment on lines +17 to +23
std::uint8_t unk00_44[0x44];
bool bGlobalDetectionOn;
std::uint8_t unk45_58[0x13];
RE::BSTArray<std::uint32_t> highActorProcessHandles;
RE::BSTArray<std::uint32_t> actorHandles68; // low, middleHigh, middleLow ??
RE::BSTArray<std::uint32_t> actorHandles78;
RE::BSTArray<std::uint32_t> actorHandles88;
Copy link
Contributor

Choose a reason for hiding this comment

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

remove RE::

Comment on lines +190 to +204

#include "RE/T/TESForm.h"
template <class From, class To>
To* Runtime_DynamicCast(RE::TESForm* a_from)
{
REL::Relocation<void*> from{ RE::detail::remove_cvpr_t<From>::RTTI };
REL::Relocation<void*> to{ RE::detail::remove_cvpr_t<To>::RTTI };

if (!from.get() || !to.get()) {
return nullptr;
}

return static_cast<To*>(RE::RTDynamicCast(const_cast<void*>(static_cast<const volatile void*>(a_from)), 0, from.get(), to.get(), false));
}

Copy link
Contributor

@qudix qudix Sep 6, 2024

Choose a reason for hiding this comment

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

what is this copy for? (almost identical to the one above)

@@ -191,7 +191,7 @@ namespace RE::BSScript

void reset();

private:
public:
Copy link
Contributor

Choose a reason for hiding this comment

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

this should not be public





Copy link
Contributor

Choose a reason for hiding this comment

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

this entire file really needs some more thinking and formatting (and namespacing)

Comment on lines +120 to +123
REL::Relocation<uintptr_t> funcPtr{ REL::ID(1868757) };
uintptr_t addr = funcPtr.address();
const auto func = reinterpret_cast<RE::BSTEventSource<TESLoadGameEvent>* (*)()>(addr);
return func();
Copy link
Contributor

@qudix qudix Sep 6, 2024

Choose a reason for hiding this comment

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

i'm not quite sure what all of these are doing. if they're just treating the address as a function ptr to call then this needs to be changed to be in line with all other existing uses. you can just call funcPtr as a function as long as Relocation<T> is callable, for example UI::IsMenuOpen

    bool IsMenuOpen(const BSFixedString& a_name)
    {
	    using func_t = decltype(&UI::IsMenuOpen);
	    REL::Relocation<func_t> func{ ID::UI::IsMenuOpen };
	    return func(this, a_name);
    }

@LarannKiar
Copy link
Author

Oh never mind, sorry. I didn't want to waste anyone's time on this. I just received a PM elsewhere I should "open a pull request with my changes"... really shouldn't have listened to them.. this code is mostly from a different environment and mixing the two isn't a good idea. You can merge the code you're interested in to the main branch of course, I'm closing this request.

@LarannKiar LarannKiar closed this Sep 6, 2024
@GELUXRUM
Copy link

GELUXRUM commented Sep 7, 2024

The point of the comment Snapdragon left under your plugin was because the licence on CLibSF requires you to post the source of your plugins and the modified libraries. Not listening to him would have been going against the agreement you make by using CLibSF to create plugins. The plugin sources should be up-to-date with the plugin as well.

Now, I'm not trying to shame or tell you off, but your plugins were made using the contributions he made to the CLibSF, and it'd only be right to return the favour and publish the findings you made. While it may seem daunting at first, learning how to integrate your changes into CLibSF the proper way would be most beneficial to everyone; the comments made here by Qudix are there to tell you what's right and wrong so that this can be fixed and learned from for future contributions.

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

Successfully merging this pull request may close these issues.

3 participants