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(modules): Scaleform class #584

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

Mycroft-Studios
Copy link

Description

Introduces a scaleform class, based upon ox lib classes, to make scaleforms both easier to read and use

Checklist

  • I have personally loaded this code into an updated Qbox project and checked all of its functionality.
  • My pull request fits the contribution guidelines & code conventions.

Introduces a scaleform class, based upon  ox lib classes, to make scaleforms both easier to read and use
modules/scaleform.lua Outdated Show resolved Hide resolved
modules/scaleform.lua Show resolved Hide resolved
modules/scaleform.lua Outdated Show resolved Hide resolved
modules/scaleform.lua Outdated Show resolved Hide resolved
modules/scaleform.lua Outdated Show resolved Hide resolved
modules/scaleform.lua Outdated Show resolved Hide resolved
modules/scaleform.lua Outdated Show resolved Hide resolved
modules/scaleform.lua Outdated Show resolved Hide resolved
Copy link
Member

@Manason Manason left a comment

Choose a reason for hiding this comment

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

I'm confused by this API. I think it would help if all the methods of this class have a leading verb in the name. I also don't think this API is abstracting away enough of the lower level complexities of scaleform. Focus the API on how the user wants to interact with scaleform, and hide the way it actually works.

For an example API that does do some abstractions, see this C# scaleform class https://github.com/citizenfx/fivem/blob/master/code/client/clrcore/External/Scaleform.cs

@Mycroft-Studios
Copy link
Author

Mycroft-Studios commented Sep 26, 2024

I also don't think this API is abstracting away enough of the lower level complexities of scaleform

may i ask how, see:
https://cdn.discordapp.com/attachments/1012753554072674398/1288912766802989067/image.png?ex=66f6e90e&is=66f5978e&hm=16f102e273bf36f2c817350d7a7a11977016fed0686ecb10231e28f8328ba4dc&

for how this looks in code. it cannot get more simplier while still giving all scaleform functionality to the user

from my latest commit, the only one without a verb, is method, which really doesnt need one

modules/scaleform.lua Outdated Show resolved Hide resolved
modules/scaleform.lua Outdated Show resolved Hide resolved
modules/scaleform.lua Outdated Show resolved Hide resolved
modules/scaleform.lua Outdated Show resolved Hide resolved
modules/scaleform.lua Outdated Show resolved Hide resolved
modules/scaleform.lua Outdated Show resolved Hide resolved
modules/scaleform.lua Outdated Show resolved Hide resolved
modules/scaleform.lua Outdated Show resolved Hide resolved
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